1
\$\begingroup\$

I am making an object formatter for use when debugging.

Formatted class:

package com.myname.somepackage;

import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;

// Allows a variable to be displayed when using Formatter.format
@Retention(RetentionPolicy.RUNTIME)
public @interface Formatted {

}

Formatter class:

package com.myname.somepackage;

import java.lang.reflect.Field;

public final class Formatter {
    private Formatter() {}

    // Returns a string containing the object's information, for debugging
    // Format: ClassName[var1=somevalue, var2=somevalue]
    // The object's variables must have the Formatted annotation to be displayed here
    public static String format(Object object) {
        String className = object.getClass().getSimpleName();
        Field[] fields = object.getClass().getDeclaredFields();
        String string = className + "[";
        for (Field field : fields) {
            field.setAccessible(true);
            Formatted annotation = field.getAnnotation(Formatted.class);
            if (annotation != null) {
                String varName = field.getName();
                try {
                    String value = field.get(object).toString();
                    string += varName + "=" + value + ", ";
                } catch (IllegalAccessException e) {
                    e.printStackTrace();
                    string += varName + "=" + "{Unavailable}, ";
                }
            }
        }
        // remove last ", "
        if (string.endsWith(", "))
            string = string.substring(0, string.length() - 2);
        string += "]";
        return string;
    }
}

A class for testing this:

package com.myname.somepackage.math.geom.r2;

import com.myname.somepackage.Formatted;
import com.myname.somepackage.Formatter;

public final class Point2d {
    @Formatted
    private final double x, y;

    public Point2d(double x, double y) {
        this.x = x;
        this.y = y;
    }

    public double getX() {
        return this.x;
    }
    public Point2d setX(double x) {
        return new Point2d(x, this.y);
    }
    public double getY() {
        return this.y;
    }
    public Point2d setY(double y) {
        return new Point2d(this.x, y);
    }

    @Override
    public String toString() {
        return Formatter.format(this);
    }
}

The code to test it:

Point2d point = new Point2d(4, 2);
System.out.println(point);

The console then outputs "Point2d[x=4.0, y=2.0]".

How does my code look? I understand reflection is considered bad, but this is just my lazy way of quickly debugging. Thanks

\$\endgroup\$
  • 1
    \$\begingroup\$ If the field's value is null, you code will fail with NullPointerException due to unchecked toString() on the get-result. \$\endgroup\$ – mtj Feb 23 '18 at 6:51
  • 1
    \$\begingroup\$ I've added the "reinventing-the-wheel" tag because the question and the approach are nice, but... Lombok library provides exactly the same functionality out of the box with @ToString annotation. \$\endgroup\$ – Antot Feb 23 '18 at 11:46
3
\$\begingroup\$

It's pretty nice IMO but can be improved.

If your project uses apache commons (this library is often included), you should consider using the FieldUtils class to get the fields : https://commons.apache.org/proper/commons-lang/apidocs/org/apache/commons/lang3/reflect/FieldUtils.html

Notably, the getFieldsWithAnnotation method would reduce your code complexity by a bit.

That's up to you though ;)

This part :

try {
    String value = field.get(object).toString();
    string += varName + "=" + value + ", ";
} catch (IllegalAccessException e) {
    e.printStackTrace();
    string += varName + "=" + "{Unavailable}, ";
}

may fail if your field is null, you should exploit the String + operator to avoid it like this :

string += varName + "=" + field.get(object) + ", ";

string is really a bad name for your variable, maybe rename it as res or something ?

I'm no big fan of the printStackTrace, you should consider using the various logging utilities proposed by java : https://docs.oracle.com/javase/9/docs/api/java/util/logging/Logger.html or slf4j.

I think those 4 modifications will already make the code neater but we can do a bigger refactoring :
instead of using a String that we concatenate bit by bit and then remove the final comma, you should consider using a Stream over the fields array and generating the result with the Collectors#joining method.

In the end, you'd have the following method :

private static final String SEPARATOR = ", ";

public static String format(Object object) {
    final String className = object.getClass().getSimpleName();
    final String prefix = className + "[";
    String res = Arrays.stream(FieldUtils.getFieldsWithAnnotation(object.getClass(), Formatted.class))
        .map(field -> {
        String varName = field.getName();
        try {
            return varName + "=" + field.get(object);
        } catch (IllegalAccessException e) {
            log.severe(e.toString());
            return varName + "=" + "{Unavailable}";
        }
    }).collect(joining(SEPARATOR));
    return prefix + res + "]";
}
\$\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.