Parsing structured text in Java

Question

I am quite new to Java. I have data structured like this:

tree 503(there is a null character here)040000 tree 0bfec80bc7b7aa0a50e0b373044929cb0aae278b build/
100755 blob e30876c61eb7712ad09b50593a30513d9b173805 build.xml
100755 blob 1574df4a2de5d2884f33a5d3e30ae3daa5affee1 manifest.mf
040000 tree fdab6a12a56a709212f105c9c8e31cd062d01e0a nbproject/
040000 tree 3c977009b6104310d7f3a66200f29bff8cda7576 src/
100755 blob de4bcf5c862c352531819e082204bb51ddabaa9c test_file
100755 blob 26af8cac3b104b4b6514621b1c512e353307a867 unicode.txt

I call each line of this an entry. To parse this data into an array of entries, I wrote this;

class TreeEntry {
    public String perms;
    public String type;
    public String hash;
    public String name;

    @Override
    public String toString() {
        return perms + " " + type + " " + hash + " " + name;
    }
}

private static TreeEntry[] parseTree(String hash) {
    String tree = readObject(hash);

    if (!tree.startsWith("tree")) {
        System.err.println("Given hash doesn't belong to a tree object");
        return null;
    }
    int currentArrayCapacity = 4;
    int currentEntries = 0;
    TreeEntry[] entries = new TreeEntry[currentArrayCapacity];

    // We are interested in data after null character
    int nullCharacterPosition = 0;
    while (tree.charAt(nullCharacterPosition) != '\0') {
        nullCharacterPosition++;
    }
    tree = tree.substring(nullCharacterPosition + 1);

    StringTokenizer st = new StringTokenizer(tree, "\n");
    while (st.hasMoreElements()) {
        TreeEntry e = new TreeEntry();
        StringTokenizer st2 = new StringTokenizer((String)st.nextElement(), " ");
        e.perms = (String) st2.nextElement();
        e.type = (String) st2.nextElement();
        e.hash = (String) st2.nextElement();
        e.name = (String) st2.nextElement();

        // There can be space in name
        while (st2.hasMoreElements()) {
            e.name += " " + (String) st2.nextElement();
        }
        currentEntries++;

        // To increase capacity of the array
        if (currentEntries > currentArrayCapacity) {
            currentArrayCapacity = currentArrayCapacity * 2;
            TreeEntry[] tmp = new TreeEntry[currentArrayCapacity];
            System.arraycopy(entries, 0, tmp, 0, entries.length);
            entries = tmp;
        }

        entries[currentEntries - 1] = e;
    }
    return entries;
}

How does it look as a Java code, and how does it look data structures and algorithms wise?

Answer 1

I'll first suggest you that the raw format that you use to store your data, use a different separator than a white-space. This will simplify the code since we are sure that we have unique separator.

Your TreeEntry should encapsulate his fields in getter and setter. Public field in a class is not something that you want to do in a OOP program. You should always limit the scope of your variable. Your TreeEntry class should look like :

class TreeEntry {

    private String perms;
    private String type;
    private String hash;
    private String name;

    @Override
    public String toString() {
        return perms + " " + type + " " + hash + " " + name;
    }

    public String getPerms() {
        return perms;
    }

    public void setPerms(String perms) {
        this.perms = perms;
    }

    public String getType() {
        return type;
    }

    public void setType(String type) {
        this.type = type;
    }

    public String getHash() {
        return hash;
    }

    public void setHash(String hash) {
        this.hash = hash;
    }

    public String getName() {
        return name;
    }

    public void setName(String name) {
        this.name = name;
    }
}

For the parsing part, I will use a different format than yours. I use the ; character to separate each "column" of the data. The format would look like :

tree;503(null)040000;tree;0bfec80bc7b7aa0a50e0b373044929cb0aae278b;build/
100755;blob;e30876c61eb7712ad09b50593a30513d9b173805;build.xml
100755;blob;1574df4a2de5d2884f33a5d3e30ae3daa5affee1;manifest.mf
040000;tree;fdab6a12a56a709212f105c9c8e31cd062d01e0a;nbproject/

I've changed a lot of things in the parseTree() method so I'll drop the new method and I'll explain later what are the changes and why.

private static List<TreeEntry> parseTree(String hash) {
    String tree = readObject(hash);

    if (!tree.startsWith("tree")) {
        System.err.println("Given hash doesn't belong to a tree object");
        return null;
    }

    List<TreeEntry> entries = new LinkedList<>();

    // // We are interested in data after null character
    int nullCharacterPosition = tree.indexOf("\0");
    tree = tree.substring(nullCharacterPosition + 1);

    String[] dataTree = tree.split("\n");

    for (String entry : dataTree) {
        TreeEntry e = new TreeEntry();
        String[] args = entry.split(";");
        e.setPerms(args[0]);
        e.setType(args[1]);
        e.setHash(args[2]);
        e.setName(args[3]);
        entries.add(e);
    }

    return entries;
}

If you want to find the null character, you should use indexOf, this will do the loop for you and will return the index of the first character corresponding. Since I don't have an input with a null value, I've not tested my code to find the null character. So be sure to verify this part.

I've also changed the type of entries to a List. I've choose a LinkedList since all the operation I'm doing is adding new entries, and you'll probably iterate over it later. This allow to focus on the task of parsing and I don't have to manage the array (I don't have to check the size before adding something).

I've used the spilt() method instead of the StringTokenizer. I've find it easy to operate in conjunction with a for-each loop, and you don't seems to want to work with anything than String, so split() is perfect for that. I'll point you to this question for further information on why I would not use StringTokenizer, read the second answer.

It seems rather odds to me that the parsing method is static. I don't know where the method is called, but I would have go with something like a factory or utility class to do the parsing job.

PS: This is my first review, so if this is not a good format or there is some inaccuracy let me know.