4
\$\begingroup\$

I am a beginner in programming and I have written a little program that manages a list of words or strings to examine my understanding of Java and object oriented programming. I am interested in your thoughts!

Dictionary.java

This class contains the words and the methods for manipulating the word list.

import java.io.File;
import java.io.BufferedReader;
import java.io.FileReader;
import java.io.FileWriter;
import java.io.BufferedWriter;
import java.io.IOException;
import java.io.FileNotFoundException;

public class Dictionary
{
    private String[] words;
    private File file;

    public Dictionary()
    {
        words = new String[1000];
        file = new File("words.txt");

        int i = 0;
        if (file.exists())
        {
            try
            {
                FileReader file_reader = new FileReader("words.txt");
                BufferedReader buffered_reader = new BufferedReader(file_reader);

                String word;
                while((word = buffered_reader.readLine()) != null)
                {
                    words[i] = word;
                    i++;
                }

                buffered_reader.close();
            }
            catch(FileNotFoundException e)
            {
                e.printStackTrace();
            }
            catch(IOException e)
            {
                e.printStackTrace();
            }
        }
        else
        {
            try
            {
                file.createNewFile();
            }
            catch(IOException e)
            {
                e.printStackTrace();
            }
        }

        for(;i < 1000; i++)
            words[i] = "";
    }

    public void show_words(String filter)
    {
        if (filter.equals("*"))
        {
            for(String word : words)
            {
                if(word.equals(""))
                {
                    break;
                }
                else if(!(word.equals("<[d]>")))
                {
                    System.out.println(word);
                }
            }
        }
        else
        {
            for(String word : words)
            {
                if(word.toLowerCase().contains(filter.toLowerCase()))
                {
                    System.out.println(word);
                }
                else if(word.equals(""))
                {
                    break;
                }
            }
        }
    }

    public void add_word(String word)
    {
        for(int i = 0; i < words.length; i++)
        {
            if (words[i].equals("") || word.equals("<[d]>"))
            {
                words[i] = word;
                break;
            }
        }
    }

    public void del_words(String string)
    {
        for(int i = 0; i < words.length; i++)
        {
            if (words[i].contains(string))
            {
                System.out.println("\"" + words[i] + "\" deleted");
                words[i] = "<[d]>";
            }
        }      
    }

    public void save_words()
    {
        try
        {
            FileWriter file_writer = new FileWriter("words.txt");
            BufferedWriter buffered_writer = new BufferedWriter(file_writer);
            int i = 0;
            while(words[i] != "")
            {
                if(!(words[i].equals("<[d]>")))
                {
                    buffered_writer.write(words[i]);
                    buffered_writer.newLine();
                }
                i++;
            }
            buffered_writer.close();
        }
        catch(IOException e)
        {
            e.printStackTrace();
        }
    }
}

Commander.java

This class takes the commands from the user and gives them to the dictionary methods or does other stuff with it.

import java.util.Scanner;

public class Commander
{
    private Dictionary dictionary;
    private Scanner input;

    public Commander()
    {
        input = new Scanner(System.in);
        dictionary = new Dictionary();
    }

    public void init()
    {
        while (true)
        {
            System.out.print("> ");
            String command = input.nextLine();

            if(command.equals("help"))
            {
                help();
            }
            else if(command.length() >= 4 && command.substring(0, 4).equals("show"))
            {
                dictionary.show_words(parse(command));
            }
            else if(command.length() >= 3 && command.substring(0, 3).equals("add"))
            {
                dictionary.add_word(parse(command));
            }
            else if(command.length() >= 3 && command.substring(0, 3).equals("del"))
            {
                dictionary.del_words(parse(command));
            }
            else if(command.equals("save"))
            {
                dictionary.save_words();
            }

            else if(command.equals("exit"))
            {
                dictionary.save_words();
                break;
            }
            else
            {
                System.out.println("invalid syntax");
            }
        }
    }

    private String parse(String input)
    {
        String parsed = input.substring(input.indexOf(' ') + 1, input.length());
        return parsed;
    }

    private void help()
    {

        System.out.println("help - no arguments    - shows information");
        System.out.println("save - no arguments    - writing changes on hard drive");
        System.out.println("show - <string> or <*> - shows words");
        System.out.println("add  - <string>        - adds word to dictionary");
        System.out.println("del  - <string>        - deletes entrys that contain string");
    }
}

Start.java

public class Start
{
    public static void main(String[] args)
    {
        Commander commander = new Commander();
        commander.init();
    }
}
\$\endgroup\$
4
\$\begingroup\$

Limit scope

    private File file;

This doesn't need to be an object field. You only use it in the constructor, so it could just be a local variable.

Choose the right data type

    private String[] words;

and later

        words = new String[1000];

These could just be

    private List<String> words = new ArrayList<>();

Then you could get change

                    words[i] = word;
                    i++;

to just

                    words.add(word);

Get rid of

        for(;i < 1000; i++)
            words[i] = "";

And get rid of the various places you say things like

                else if(word.equals(""))
                {
                    break;
                }

The list will only be as long as needed, so it doesn't have to handle dummy values.

            if (words[i].contains(string))
            {
                System.out.println("\"" + words[i] + "\" deleted");
                words[i] = "<[d]>";
            }

could be

            while (i < words.size() && words.get(i).contains(string))
            {
                System.out.println("\"" + words.remove(i) + "\" deleted");
            }

Now we don't have fake values hiding in the dictionary. So we don't need code like

                if(word.equals(""))
                {
                    break;
                }
                else if(!(word.equals("<[d]>")))
                {
                    System.out.println(word);
                }

We can just say

                System.out.println(word);

And

        for(int i = 0; i < words.length; i++)
        {
            if (words[i].equals("") || word.equals("<[d]>"))
            {
                words[i] = word;
                break;
            }
        }

could just be

        words.add(word);

No need to find a blank spot.

try-with-resources

            try
            {
                FileReader file_reader = new FileReader("words.txt");
                BufferedReader buffered_reader = new BufferedReader(file_reader);

                String word;
                while((word = buffered_reader.readLine()) != null)
                {
                    words[i] = word;
                    i++;
                }

                buffered_reader.close();
            }

If there is an exception, this never closes the buffered_reader.

            try (BufferedReader buffered_reader = new BufferedReader(new FileReader("words.txt")))
            {
                String word;
                while ((word = buffered_reader.readLine()) != null)
                {
                    words.add(word);
                }
            }

With the try-with-resources form, the buffered_reader will automatically be closed.

Command processing

        while (true)
        {
            System.out.print("> ");
            String command = input.nextLine();

            if(command.equals("help"))
            {
                help();
            }
            else if(command.length() >= 4 && command.substring(0, 4).equals("show"))
            {
                dictionary.show_words(parse(command));
            }
            else if(command.length() >= 3 && command.substring(0, 3).equals("add"))
            {
                dictionary.add_word(parse(command));
            }
            else if(command.length() >= 3 && command.substring(0, 3).equals("del"))
            {
                dictionary.del_words(parse(command));
            }
            else if(command.equals("save"))
            {
                dictionary.save_words();
            }

            else if(command.equals("exit"))
            {
                dictionary.save_words();
                break;
            }
            else
            {
                System.out.println("invalid syntax");
            }
        }

This could be a switch

        while (true)
        {
            System.out.print("> ");
            String[] command_pieces = input.nextLine().split("\\s+", 2);

            switch (command_pieces[0]) {
            case "help":
                help();
                break;
            case "show":
                dictionary.show_words(command_pieces[1]);
                break;
            case "add":
                dictionary.add_word(command_pieces[1]);
                break;
            case "del":
                dictionary.del_words(command_pieces[1]);
                break;
            case "save":
                dictionary.save_words();
                break;
            case "exit":
                dictionary.save_words();
                return;
            default:
                System.out.println("invalid syntax");
            }
        }

This saves some typing, as using split with nextLine means that you don't have to take the substring or check that the string is long enough.

Help

You should include exit in your help instructions. So people know it's exit and not quit or stop or something else. You could also explain that exit includes a save operation. So people don't think they have to type save every time.

Advanced

You could pick a type that better optimizes substring searches, like a modified Trie or a full text index. Even something like a HashSet would help with full string matches.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thank you very much for your work! This will help me a lot for improving my programming skills! \$\endgroup\$ – Dexter Thorn Jun 14 '17 at 6:23

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.