mercredi 6 février 2019

Should i pass a Singleton as a function parameter

I recently encountered in a code review i was doing to a colleague, in a peculiar pattern i haven't seen before. He was sending a singleton as a parameter in a function and saved it in the calling class as a member. Let's say i have a simple singleton class Foo

public class Foo {

private static Foo instance;
private int counter;

private Foo(){}

public static Foo getInstance(){
    if (instance == null){
        instance = new Foo();
    }
    return instance;
}

public int getCounter() {
    return counter;
}

public void setCounter(int counter) {
    this.counter = counter;
}
}

Now there's a client of Foo called Bar:

public class Bar {

private Foo foo;

public Bar(Foo foo){
    this.foo = foo;
    this.foo.setCounter(10);
    this.foo.setCounter(20);
}

public int getCounter(){
    return foo.getCounter();
}
}

So Bar does 2 unusual things - 1. Call Foo in the constructor, instead of just using Foo.getInstance() and 2. Save Foo as a member so to 'save' the boilerplate of calling Foo.getInstance() it uses it. Of course the usages i demonstrated are trivial.

This looked odd and awkward to me, and worse, i can't identify Foo as a singleton at all in Bar scope. What if i change something crucial in it's state? But other than that reason of code-readability, i couldn't say that it's not optional to do so, and the usage seems eventually valid. Was i right or wrong? and for what reason? is it right to pass this singleton and save it as a member?

Aucun commentaire:

Enregistrer un commentaire