HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Getting a list of all fields in an object

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
allfieldsgettinglistobject

Problem

I've got the following code that I use to output a list of all (public) fields in an object in an easy(ish) to read way. The issue with it is that the code is not easy to look at, and I'm not sure what I could do to improve it.

I'm not too fond of all of the StringBuilders, the append calls are ugly and chaining them does not help the situation however I can't see a better way of doing it.

public abstract class Component {
    private String[] getFields() {
        Class componentClass = getClass();
        List lines = new ArrayList<>();

        for (Field field: componentClass.getFields()) {
            StringBuilder lineBuilder = new StringBuilder();

            lineBuilder.append(field.getName());

            field.setAccessible(true);

            try {
                Object value = field.get(this);

                lineBuilder.append(" = ");
                lineBuilder.append(value);
            } catch (IllegalAccessException e) {
                lineBuilder.append(" > ");
                lineBuilder.append(e.getClass().getSimpleName());
            }

            lines.add(lineBuilder.toString());
        }

        return lines.toArray(new String[lines.size()]);
    }

    @Override
    public final String toString() {
        StringBuilder builder = new StringBuilder(getClass().getSimpleName());
        boolean firstIteration = true;

        builder.append('(');

        for (String field: getFields()) {
            if (!firstIteration) {
                builder.append(", ");
            } else {
                firstIteration = false;
            }

            builder.append(field);
        }

        builder.append(')');

        return builder.toString();
    }
}


I'm not entirely sure why I separated getFields into it's own method, however it might be better to move it back into toString.

Solution

If you are using Java 8, you can do:

private String[] getFields() {
    Class componentClass = getClass();
    Field[] fields = componentClass.getFields();
    List lines = new ArrayList<>(fields.length);

    Arrays.stream(fields).forEach(field -> {
        field.setAccessible(true);
        try {
            lines.add(field.getName() + " = " + field.get(this));
        } catch (final IllegalAccessException e) {
            lines.add(field.getName() + " > " + e.getClass().getSimpleName());
        }
    });

    return lines.toArray(new String[lines.size()]);
}


or to improve formatting:

private String[] getFields() {
    Class componentClass = getClass();
    Field[] fields = componentClass.getFields();
    List lines = new ArrayList<>(fields.length);

    Arrays.stream(fields)
            .forEach(
                    field -> {
                        field.setAccessible(true);
                        try {
                            lines.add(field.getName() + " = " + field.get(this));
                        } catch (final IllegalAccessException e) {
                            lines.add(field.getName() + " > "
                                    + e.getClass().getSimpleName());
                        }
                    });

    return lines.toArray(new String[lines.size()]);
}


Note that this is adding to @EricStein's excellent answer on normal concatenation instead of StringBuilders.

(I'm just starting to look into Streams, so if this is bad, please comment on why)

Otherwise...

Just a small comment:

lineBuilder.append(" = ");
lineBuilder.append(value);


can easily be:

lineBuilder.append(" = ").append(value);


Same with:

lineBuilder.append(" > ");
lineBuilder.append(e.getClass().getSimpleName());


becomes:

lineBuilder.append(" > ").append(e.getClass().getSimpleName());


Also, I would use a simple array instead of an ArrayList:

private String[] getFields() {
    Class componentClass = getClass();
    Field[] fields = componentClass.getFields();
    String[] lines = new String[fields.length];

    int index = 0;
    for (Field field : fields) {
        StringBuilder lineBuilder = new StringBuilder();

        lineBuilder.append(field.getName());

        field.setAccessible(true);

        try {
            Object value = field.get(this);

            lineBuilder.append(" = ");
            lineBuilder.append(value);
        } catch (IllegalAccessException e) {
            lineBuilder.append(" > ");
            lineBuilder.append(e.getClass().getSimpleName());
        }

        lines[index++] = lineBuilder.toString();
    }

    return lines;
}


EDIT: The above review, if not using Java 8, should be replaced by @EricStein's code.

Also, getFields() is a bad name. Try getFieldDescriptions().

Code Snippets

private String[] getFields() {
    Class<? extends Component> componentClass = getClass();
    Field[] fields = componentClass.getFields();
    List<String> lines = new ArrayList<>(fields.length);

    Arrays.stream(fields).forEach(field -> {
        field.setAccessible(true);
        try {
            lines.add(field.getName() + " = " + field.get(this));
        } catch (final IllegalAccessException e) {
            lines.add(field.getName() + " > " + e.getClass().getSimpleName());
        }
    });

    return lines.toArray(new String[lines.size()]);
}
private String[] getFields() {
    Class<? extends Component> componentClass = getClass();
    Field[] fields = componentClass.getFields();
    List<String> lines = new ArrayList<>(fields.length);

    Arrays.stream(fields)
            .forEach(
                    field -> {
                        field.setAccessible(true);
                        try {
                            lines.add(field.getName() + " = " + field.get(this));
                        } catch (final IllegalAccessException e) {
                            lines.add(field.getName() + " > "
                                    + e.getClass().getSimpleName());
                        }
                    });

    return lines.toArray(new String[lines.size()]);
}
lineBuilder.append(" = ");
lineBuilder.append(value);
lineBuilder.append(" = ").append(value);
lineBuilder.append(" > ");
lineBuilder.append(e.getClass().getSimpleName());

Context

StackExchange Code Review Q#108222, answer score: 5

Revisions (0)

No revisions yet.