2

Today I saw the following JavaFX related code in a project:

issueComboBox.setConverter(new IntegerStringConverter());
yearComboBox.setConverter(new IntegerStringConverter());

and thought: Do I really need to create two instances of IntegerStringConverter?

IntegerStringConverter has the following code:

public class IntegerStringConverter extends StringConverter<Integer> {

    @Override public Integer fromString(String value) {
        if (value == null) {
            return null;
        }

        value = value.trim();

        if (value.length() < 1) {
            return null;
        }

        return Integer.valueOf(value);
    }

    @Override public String toString(Integer value) {

        if (value == null) {
            return "";
        }

        return (Integer.toString(((Integer)value).intValue()));
    }
}

So I don't need two instances, because there is no state in IntegerStringConverter, also not in StringConverter. If I could rewrite the class I would rewrite it as a singleton, like that:

public final class IntegerStringConverter extends StringConverter<Integer> {

    public static final IntegerStringConverter INSTANCE = new IntegerStringConverter();

    private IntegerStringConverter() { }

    @Override public Integer fromString(String value) {
        if (value == null) {
            return null;
        }

        value = value.trim();

        if (value.length() < 1) {
            return null;
        }

        return Integer.valueOf(value);
    }

    @Override public String toString(Integer value) {

        if (value == null) {
            return "";
        }

        return (Integer.toString(((Integer)value).intValue()));
    }
}

So users of this class can't create multiple instances:

issueComboBox.setConverter(IntegerStringConverter.INSTANCE);
yearComboBox.setConverter(IntegerStringConverter.INSTANCE);

Its a class from javafx.util.converter package. I think it isn't implemented in the way i suggested because the JavaFX developers want to give us the possibility to extend this class.

The point of the matter is, is it nearly always a good idea to implement classes ( which are not pure helper classes with only static methods) with no state as singletons to prevent multiple instantiations for memory and performance reasons?

user2867869
  • 183
  • 6
  • 2
    How about implementing those methods as static? – gunar Jan 30 '14 at 14:51
  • 1
    @gunar: this is probably not an option, because you have to implement all methods of `StringConverter` which could not be static! – bobbel Jan 30 '14 at 15:19

2 Answers2

1

I can't exactly say why exactly was it done that way in JavaFX.

One possible explanation can be that Singleton is bad for inheritance. You can extend IntegerStringConvertor in future to provide more specific implementation.

Narendra Pathai
  • 41,187
  • 18
  • 82
  • 120
0

If no method of your IntegerStringConverter class will change any values of the members in this class, you could pass it as a singleton. In your case it could be done like this, because you don't even have a member in your class

But imagine, you have a member field in this class which will be modified due to method calls, this is not a good idea. It's like the point, that you have to be thread safe in this case. This is like the question, why not using a single instance of SimpleDateFormat for multiple calls. Because it is not thread safe! See: Why is Java's SimpleDateFormat not thread-safe?

So, this would be a bad idea:

public class Foo {
    public static final Foo INSTANCE = new Foo();

    private int value = 0;

    public void changeIt() {
        value++;
    }

    public int getValue() {
        return value;
    }
}

public class PrintThingy {
    private static void print(Foo foo) {
        foo.changeIt();
        System.out.println(foo.getValue());
    }

    public static void main(String[] args) {
        print(Foo.INSTANCE);  // prints 1
        print(Foo.INSTANCE);  // prints 2 - huh!
    }
}

So, it really depends on what you're doing in this singleton class and what will be done in the methods you're passing your singleton instance through!

Community
  • 1
  • 1
bobbel
  • 3,327
  • 2
  • 26
  • 43