patternjavaMinor
Getting a list of all fields in an object
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
I'm not entirely sure why I separated
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:
or to improve formatting:
Note that this is adding to @EricStein's excellent answer on normal concatenation instead of
(I'm just starting to look into
Otherwise...
Just a small comment:
can easily be:
Same with:
becomes:
Also, I would use a simple array instead of an
EDIT: The above review, if not using Java 8, should be replaced by @EricStein's code.
Also,
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.