Refactor ConvertorToString class Help me refactor this class that helps to represent an object state as String:

public class ConvertorToString {
    private static final String SEPARATOR_BETWEEN_FIELD_NAME_AND_VALUE = "=";
    private static final String SEPARATOR_BETWEEN_FIELDS = ", ";
    private static final String OPEN_FIELDS = "( ";
    private static final String CLOSE_FIELDS = " )";

    private final String className;
    private String[] fieldNames;
    private Object[] fieldValues;

    public static ConvertorToString buildFor(Object someObject) {
        return new ConvertorToString(someObject.getClass().getName());
    }

    private ConvertorToString(String className) {
        this.className = className;
    }

    public ConvertorToString withFieldNames(String... fieldNames) {
        this.fieldNames = fieldNames;
        return this;
    }

    public ConvertorToString withFieldValues(Object... fields) {
        this.fieldValues = fields;
        return this;
    }

    private class Field {
        private final int index;

        public Field(int index) {
            this.index = index;
        }

        public String toString() {
            checkIndex(index);
            return fieldNames[index] + SEPARATOR_BETWEEN_FIELD_NAME_AND_VALUE + fieldValues[index];
        }

        private void checkIndex(int index) {
            if (index &lt; 0 || index &gt;= countOfFields()) {
                throw new IllegalArgumentException("Index should be inside a range: " +
                        "[0," + countOfFields() + "];  " + "but index was: " + index);
            }
        }
    }

    public String toString() {
        checkState();
        return className
               + OPEN_FIELDS +
                StringsCombiner.combine(getFields(), SEPARATOR_BETWEEN_FIELDS)
               + CLOSE_FIELDS;
    }

    private void checkState() {
        if (fieldNames == null || fieldValues == null || fieldNames.length != fieldValues.length) {
            throw new IllegalStateException("count of fieldNames should be equal to count of fieldValues");
        }
    }

    private List&lt;Field&gt; getFields() {
        List&lt;Field&gt; fields = new ArrayList&lt;Field&gt;(countOfFields());
        int countOfFields = countOfFields();
        for (int i = 0; i &lt; countOfFields; ++i) {
            fields.add(new Field(i));
        }
        return fields;
    }

    private int countOfFields() {
        return fieldNames.length;
    }
}


Using:

class SomeClass {
    int id;
    String title;

    @Override
    public String toString() {
        return ConvertorToString.buildFor(this)
            .withFieldNames("id", "title")
            .withFieldValues(id, title)
            .toString();
    }
}
I like the idea of this utility class, but there are a few points I would like to mention.

Let's start with naming. I would rename the ConvertorToString class, probably, to one of the following ToStringUtil, ToStringHelper, ObjectToString or ObjectToStringConvertor. The rule of thumb is to put the -or/-er nouns in the end of the class name. Personally, I like the first two. Then come the constant fields which could be shorter. For instance, they could be named as FIELD_NAME_AND_VALUE_SEPARATOR instead of SEPARATOR_BETWEEN_FIELD_NAME_AND_VALUE and FIELDS_SEPARATOR instead of SEPARATOR_BETWEEN_FIELDS. That is, the 'BETWEEN' part is redundant because it's already obvious that a separator separates the one from the other. Two other fields would be better named as FIELDS_START and FIELDS_END, which is more intuitive.

Another thing is that you have two separate methods for adding the field names and values and then you check that there is a corresponding value for each field name in the ConvertorToString.checkState() method. You also check for indexes and arrays' length in the ConvertorToString.Field.checkIndex() method. All these checks are error-prone, but they could be eliminated by enforcing the one-to-one correspondence rule of the field names and values in a single method for adding those fields and their values:

ToStringUtil.addField(String name, Object value);
ToStringUtil.addFields(Map&lt;String, Object&gt; fields);


This way you control the state of your object and have no need in the ConvertorToString.checkState() and ConvertorToString.Field.checkIndex() methods anymore. Moreover, internally I would store the field names and values in one data structure, rather than in two separate arrays. The single structure could be a Map&lt;String, Object&gt; or a List&lt;Field&gt;, the latter one is preferable if you have a Field class. This brings us to the ConvertorToString.Field class.

I think, it would be more practical to encapsulate the field names and values within the ConvertorToString.Field class, which will yield a better OOP-style design. Also, this change will allow you to transform the ConvertorToString.Field class from an inner class to a static nested class, because you don't need the reference to the outer class anymore. After you encapsulate the field name and its value in the ConvertorToString.Field class, the FIELD_NAME_AND_VALUE_SEPARATOR field would naturally belong to the ConvertorToString.Field class. And because the class name is Field, the FIELD_NAME_AND_VALUE_SEPARATOR field could further be renamed to NAME_AND_VALUE_SEPARATOR.

One other opinion on the ConvertorToString constants would be that you don't actually need them because you don't use them anywhere else besides the toString() methods. So, you could just use the string literals instead:

public class ToStringConvertor {
    private final String className;
    private final List&lt;Field&gt; fields;
    ...
    @Override
    public String toString() {
        return String.format(
            "%s(%s)", 
            className, 
            StringsCombiner.combine(fields, ", ")
        );
    }

    private static class Field {
        private final String name;
        private final Object value;
        ...
        @Override
        public String toString() {
            return String.format("%s=%s", name, value);
        }
    }
}