6
\$\begingroup\$

I was assigned a task to make a simple jar that can handle 2 console commands -> create key value, read key. I have 4 classes, where my main logic is implemented. Eg. create bulgaria sofia, creates a record in a serialized file with a key - bulgaria and value - sofia. Any feedback about the code and the idea are more than welcome.

            public class MapUtility {

                public static final String FILE_NAME = "hashmap";

                // Finds a value based on its key
                public static String findByKey(String key) throws ExceptionUtil,
                        IOException {
                    Map<String, String> map = deserializeMap(OSUtility
                            .getFilePathForSerialization(FILE_NAME));

                    for (String k : map.keySet()) {
                        if (key.equals(k))
                            return map.get(k);
                    }

                    throw new ExceptionUtil(1);
                }

                public static Map<String, String> serializeMap(String key, String value,
                        String fileName) throws IOException {

                    String pathToMap = OSUtility.getFilePathForSerialization(fileName);
                    Map<String, String> map = deserializeMap(pathToMap);

                    if (map == null) {
                        map = new TreeMap();
                    }

                    map.put(key, value);

                    try (FileOutputStream fois = new FileOutputStream(pathToMap);
                            ObjectOutputStream oos = new ObjectOutputStream(fois)) {
                        oos.writeObject(map);
                    } catch (IOException ioe) {
                        ioe.printStackTrace();
                    }

                    return map;
                }

                // Deserialize a map object
                public static Map<String, String> deserializeMap(String pathToMap) {
                    Map<String, String> map = null;

                    try (FileInputStream fis = new FileInputStream(pathToMap);) {
                        int numberOfBytesForReading = fis.available();
                        if (numberOfBytesForReading > 0) {
                            try (ObjectInputStream ois = new ObjectInputStream(fis);) {
                                map = (Map) ois.readObject();
                            }
                        }

                    } catch (Exception e) {
                        e.printStackTrace();
                    }

                        return map;
                    }

                }

         public class CommandsUtility {

            public static final String[] HELP_ARRAY  = {
                  "Output: Invalid arguments provided.",
                  "Run 'app' without parameters to get list of supported operations",
                  "or just help as a single argument",
            };

            public static final Map<String,ICommand> LEGITIMATE_COMMANDS;
            static
            {
                LEGITIMATE_COMMANDS = new TreeMap<String, ICommand>();
                LEGITIMATE_COMMANDS.put("create", new CreateCommand());
                LEGITIMATE_COMMANDS.put("read", new ReadCommand());
            }

            public static ICommand getCommandBasedOnKey(String value, String[] args) throws ExceptionUtil {
                ICommand command = null;
                boolean found = false;

                for(String key:LEGITIMATE_COMMANDS.keySet()) {
                    if(value.equals(key)) {
                        found = true;
                        command =  LEGITIMATE_COMMANDS.get(key);
                        break;
                    }
                }

                if(!found) throw new ExceptionUtil(1);

                String key = null;
                if(args.length == 2) {
                    //read
                    key = args[1];
                    command.setKey(key);
                }
                else  
                {
                    //create
                    key = args[1];
                    String val = args[2];
                    command.setKey(key);
                    command.setValue(val);
                }

                return command;
            }

            public static String gethHelp(String[] array) {
                StringBuilder sb = new StringBuilder();

                for(String s:array) {
                    sb.append(s + "\n");
                }

                return sb.toString();
            }
        }

        public class OSUtility {

        public static String getFilePathForSerialization(String nameOfFile) throws IOException   { 
            File serializFile = new File(System.getProperty("user.dir"), nameOfFile + ".ser" );

            if(!serializFile.exists()) {
                    serializFile.createNewFile();
            } 

            return serializFile.getAbsolutePath();
        }
    }

    public class ValidationUtility {

        public static boolean validateInput(String[] input)  {
            int minRequiredArgs = CommandsUtility.LEGITIMATE_COMMANDS.keySet()
                    .size();

            if (input.length == minRequiredArgs || input.length == minRequiredArgs + 1) {
                String command = input[0];

                for (String legitimateCommand : CommandsUtility.LEGITIMATE_COMMANDS
                        .keySet()) {
                    if (command.equals(legitimateCommand))
                        return true;
                }
            }


            return false;
        }

    }


    public interface ICommand {
        public void execute();
        public void setValue(String value);
        public void setKey(String key);
        public String getHelp();
    }

    public class CreateCommand implements ICommand   {

        private String value;
        private String key;
        private Map<String, String> recordMaps;

        public CreateCommand() {
            recordMaps = new TreeMap<String, String>();
        }

        @Override
        public void execute() {
            try {
                recordMaps = MapUtility.serializeMap(this.key, this.value, MapUtility.FILE_NAME);
                System.out.println(this.key);
            } catch (IOException e) {
                e.printStackTrace();
            }
        }

        @Override
        public String getHelp() {
            // arg_name for a required arg of which there can be many
            return new StringBuilder("Create command, only valid usage syntax \n")
                .append("create args_name args_name \n").append("Eg. create record1 -> stores record1 under some Id")
                .toString();
        }

        @Override
        public void setKey(String key) {
            this.key = key;
        }


        @Override
        public void setValue(String value) {
            this.value = value;
        }

    }

    public class ReadCommand implements ICommand  {

        private String key;
        private String contentByCreatedPreviousRecord;

        @Override
        public void execute() {
            Map<String, String> map;
            try {
                map = MapUtility.deserializeMap(OSUtility.getFilePathForSerialization("hashmap"));
                contentByCreatedPreviousRecord = MapUtility.findByKey(this.key);
                System.out.println(contentByCreatedPreviousRecord);
            } catch (IOException e) {
                e.printStackTrace();
            } catch (Exception e) {
                e.printStackTrace();
            }
        }

        @Override
        public void setValue(String value) {

        }

        @Override
        public String getHelp() {
            // [arg_name...] for an arg for which any number can be supplied
            return new StringBuilder("Read command, only valid usage syntax \n")
                .append("find [arg_name...] \n").append("Eg. find 3 -> retrieves the content based on id 3")
                .toString();
        }

        @Override
        public void setKey(String key) {
            this.key = key;
        }


    }



       public class CommandDistributor {
       private List<ICommand> history = new ArrayList<ICommand>();

       public void storeAndExecute(ICommand cmd) {
          this.history.add(cmd); 
          cmd.execute();
       }

    }  

   public class ExceptionUtil extends Exception {
    private static final Map<Integer, String> MAP_IDS_TO_MESSAGES_EXCEPTIONS;
    static
    {
        MAP_IDS_TO_MESSAGES_EXCEPTIONS = new TreeMap<Integer, String>();
        MAP_IDS_TO_MESSAGES_EXCEPTIONS.put(1, "That id does not exist");
    }

    private int id;

    public ExceptionUtil(int id)
    {
        super(new StringBuilder("Problem with that Id -> ").append(String.valueOf(id)).toString());
        this.id = id;
    }

    @Override
    public String getMessage()
    {   
         return MAP_IDS_TO_MESSAGES_EXCEPTIONS.get(id); 
    }
}


    public class StartingPoint {

        public static void main(String[] args) throws IOException, ExceptionUtil {
            CommandDistributor commandDistributor = new CommandDistributor();
            if (ValidationUtility.validateInput(args)) {
                String command = args[0];
                ICommand commandForExecution = 
                        CommandsUtility.getCommandBasedOnKey(command, args);

                commandDistributor.storeAndExecute(commandForExecution);
            } else {
                boolean helpExistInArgs = Arrays.asList(args).contains(
                        "help");

                if (helpExistInArgs || args.length == 0) {
                    //go through each command and display getHelp()
                    for(ICommand command:CommandsUtility.LEGITIMATE_COMMANDS.values()) {
                        System.out.println(command.getHelp());
                    }

                } else {

                    //It might be another command object, but I think
                    //that there is a slight separation between a command and that 
                    //message. Also argument could be made for consistency.
                     String helpMessage = 
                             CommandsUtility.gethHelp(CommandsUtility.HELP_ARRAY);
                     System.out.println(helpMessage);
                }

            }

        }

    }
\$\endgroup\$
  • \$\begingroup\$ Can you please make your title more specific on your application purpose? \$\endgroup\$ – Caridorc Jun 23 '15 at 10:20
  • \$\begingroup\$ Can you show your CreateCommand and ReadCommand classes as well? \$\endgroup\$ – Simon Forsberg Jun 23 '15 at 11:03
  • \$\begingroup\$ I just added the interface, commands and my starting point, I also have a custom exception, but I am not so concern about it. @Caridorc the application purpose is to extract it as a single jar in some directory that you want, and just start adding pairs in the file, or retrieve previous content. That is all, it's really simple, but it still requires thinking :) \$\endgroup\$ – Recoba20 Jun 23 '15 at 11:18
  • \$\begingroup\$ Simple code is welcome :) , I just asked you to change the title to give more precise information to potential reviewers. \$\endgroup\$ – Caridorc Jun 23 '15 at 11:25
  • \$\begingroup\$ Ok, I am sorry, usually they close my questions in 2 minutes after I create them :) I modified it, hope it is ok now. \$\endgroup\$ – Recoba20 Jun 23 '15 at 11:39
2
\$\begingroup\$

User input and validation

Instead of having your own implementation to parse user input in a CLI, consider using a third-party library such as Apache Commons-CLI to simplify the approach.

Exception names

Exception names are usually in the form of ...Exception, hence your class ExceptionUtil has an atypical name. Also, Exceptions are usually created with a known message already, and I think your approach of using a static Map that is only referenced when getMessage() is called is a bit convoluted.

Map access

I am not sure why your MapUtility.findByKey() method is so verbose, when a simple statement will suffice:

return deserializeMap(OSUtility.getFilePathForSerialization(FILE_NAME)).get(key);
| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ Thank you a lot for giving your time and efforts to provide feedback. Really good point regarding the map access, and I was thinking the same about the Exception class, but a friend of mine insisted that this is a better approach. User input and validation - to be honest I think that using third-party library for console application is not the right approach, because the goal of it is to get familiar with some key api-s, but it is still a valid note and I appreciate it one more time. \$\endgroup\$ – Recoba20 Jun 23 '15 at 17:42
1
\$\begingroup\$

I whipped up a little example of how I would do it.... Basically this is avoiding having to de-serialize an entire map object. It is sort of like a hashtable, but its "buckets" are actual files.

A real solution for persisting data really should use a database which removes a bunch of gritty implementation details. Hopefully this answer is somewhat helpful as another example of how you could potentially do this, though? Good luck!

import java.io.*;

public class KeyValueSerializer {

    public static void main(String[] args) {
        try {
            processCommand(args);
        } catch (Exception e) {
            System.out.println(e.getMessage());
            e.printStackTrace();
        }
    }
    public static void processCommand(String[] args) throws IOException, ClassNotFoundException {
        if (args.length > 0) {
            switch (args[0]) {
                case "create":
                    if (args.length != 3)
                        break;
                    serializeElement(new KeyValueElem(args[1], args[2]));
                    System.out.println("STORED:\nKEY:   " + args[1] + "\nVALUE: " + args[2]);
                    return;
                case "read":
                    if (args.length != 2)
                        break;
                    KeyValueElem deserialized;
                    deserialized = deserializeElement(args[1]);
                    System.out.println("VALUE: \"" + deserialized.value + "\"");
                    return;
                case "delete":
                    if (args.length != 2)
                        break;
                    if (deleteElement(args[1]))
                        System.out.println("Key \"" + args[1] + "\" was successfully deleted.");
                    else
                        System.out.println("The specified key was not found.");
                    return;
            }
        }
        throw new IllegalArgumentException("Usage: {create {key value}|{read|delete} {key}}.");
    }

    private static void serializeElement(KeyValueElem e) throws IOException, ClassNotFoundException {
        File file = new File(getFileName(e.key));
        LinkedList list;
        if (file.exists()) {
            list = (LinkedList)
                    new ObjectInputStream(new FileInputStream(file)).readObject();
            list.add(e);
        } else {
            if(!file.createNewFile())
                throw new IOException(); // something went wrong
            list = new LinkedList(e);    // create a new list.
        }
        new ObjectOutputStream(new FileOutputStream(file)).writeObject(list);
    }

    private static KeyValueElem deserializeElement(String key) throws IOException, ClassNotFoundException {
        File file = new File(getFileName(key));
        LinkedList list;
        if (file.exists()) {
            list = (LinkedList)
                    new ObjectInputStream(new FileInputStream(file)).readObject();
            return list.getKey(key);
        }
        return null;
    }

    private static boolean deleteElement(String key) throws IOException, ClassNotFoundException {
        File file = new File(getFileName(key));
        LinkedList list;
        if (file.exists()) {
            list = (LinkedList)
                    new ObjectInputStream(new FileInputStream(file)).readObject();
            boolean success = list.remove(key);
            new ObjectOutputStream(new FileOutputStream(file)).writeObject(list);
            return success;
        }
        return false;
    }

    private static String getFileName(String key) {
        return Integer.toHexString(Math.abs(key.hashCode()) % 193) + ".dat";
    }

    private static class LinkedList implements Serializable {
        private KeyValueElem head;
        private KeyValueElem tail;
        public LinkedList(KeyValueElem head) {
            this.head = head;
            this.tail = head;
        }
        public KeyValueElem getKey(String key) {
            KeyValueElem current = head;
            while (current != null && !current.key.equals(key))
                current = current.next;
            return current;
        }
        public void add(KeyValueElem e) {
            if (head == null) {
                head = e; tail = e;
                return;
            }
            KeyValueElem added = getKey(e.key);
            if (added != null) {
                if (!added.value.equals(e.value))
                    throw new RuntimeException("Specified key already exists!");
            } else {
                tail.next = e;
                tail = e;
            }
        }
        public boolean remove(String key) {

            if (head == null)
                return false;

            KeyValueElem current = head;
            if (current.key.equals(key)) {
                head = head.next;
                return true;
            }

            while (current.next != null && !current.next.key.equals(key))
                current = current.next;

            if (current.next == null) {
                return false;
            } else if (current.next.next == null) {
                current.next = null;
                tail = current;
            } else {
                    current.next = current.next.next;
            }
            return true;
        }
    }

    private static class KeyValueElem implements Serializable {
        public KeyValueElem next;
        public final String key;
        public final String value;
        public KeyValueElem(String key, String value) {
            this.key = key;
            this.value = value;
        }
    }
}

note that a lot of people like to use "try with resources" with the output/input streams, but I figure since this is so small it doesn't really matter here.

| improve this answer | |
\$\endgroup\$

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.