0

I would like to know if this kind of design is right, wrong or both.
I have an app with several components (objects), each of them have a configuration (flags, capabilities, etc.) At any time, I would like to retrieve the configs from the objects.
To so, I did this:

class ConfigRetriever {
    public String getConfigString() {
        String configString = "";   
        configString += "component 1 flag : "+component1.getFlag()+"\n";
        configString += "component 2 flag : "+component2.getFlag()+"\n";
        // ...
        return( configString );
    }
}

Somewhere else, when config is needed :

class SomeClass {
    public void someMethod() {
        ConfigRetriever configRetriever = new ConfigRetriever(); 
        String configString = configRetriever.getConfigString();
            // Do some stuff with configString...
    }
}

I am quite new in object programming and it still feels weird to create an object (ConfigRetriever) for just one specific action (even if the object is able to do other stuff).
I also though about the singleton pattern, with some thing like this :

String configString = ConfigRetriever.getInstance().getConfigString();

It's a neat line, but since the object stays in memory until the end of the app, I don't really know what is right and what is wrong.

Is my design could be better ? How ? Why ?

UPDATE
Thank you for your answers. I think my question was a bit messy and I missed the point about what I was asking for.
The whole config and components story is here as an example of the kind of situation I am dealing with. This is quick and dirty code and I should have warn you about that. The real question was : "is it good to create an object just once (or sometimes) to access to one of its methods ?" Well, reading your answers and thinking again about it, it seems the right answer would be "it depends of your goal, the classes, the responsabilities and so on..."
Do I want to store informations in my objet ? Can't be static method.
Is a singleton with permanent memory using is a problem ? Most of the time it is, because I think you have to get a good reason to maintain an object in memory with a global state. So, most of the time : no singleton.

Finally, is it a problem to create a class to be used to instantiate an object sometime ? It is not, go for it! :-)

Antoine
  • 480
  • 1
  • 4
  • 15
  • I'm not seeing anything in your question that suggests singleton except you suggesting it. Seems to be something missing though, where did component1 and component2 come from? – Tony Hopkinson Sep 28 '12 at 17:09

3 Answers3

1

I don't see any need to have a singleton here.

It's not weird to create object for a single operation, but it could be not efficient if you call that method frequently. I think the best you can do is to use dependency injection:

class SomeClass {
    private final ConfigRetriever retriever;
    public SomeClass(ConfigRetriever retriever) {
        this.retriever = retriever;
    }
    public void someMethod() {
        // use this.retriever here
    }
}
Community
  • 1
  • 1
nogard
  • 9,432
  • 6
  • 33
  • 53
1

No one has mentioned static methods yet. A typical Java pattern is to use a static method which you class without having an instance of the class. Something like:

class ConfigRetriever {
   public static String getConfigString() {
      StringBuilder sb = new StringBuilder();
      sb.append("component 1 flag : ").append(component1.getFlag()).append('\n');
      sb.append("component 2 flag : ").append(component2.getFlag()).append('\n');
      // ...
      return sb.toString();
   }
}

So this allows you to do something like:

// call the static method on the class, not on an instance
String configString = ConfigRetriever.getConfigString();

Since you don't have an instance, you can't store state inside of the ConfigRetriever. I'm not sure where the component1 and component2 objects come from then.

Notice that I converted your getConfigString() method to use the StringBuilder() class which is much more efficient than the += method which actually uses multiple StringBuilder classes internally.

Gray
  • 115,027
  • 24
  • 293
  • 354
0

Several issues -
A. Your original code has a method returning void named getConfigString - this should be fixed.
B. In addition, it is not clear where the component is provided to in the ConfigRetriever class.
C. You have some options you can choose from:

1. Have ConfigProvider class hierarchy, that will match your Component hierarchy -
If you have class named ComputerComponent you will have a matching ComptuerConfigProider.
You may consider having a ConfigProider getConfigProvider method at the Component class.

public abstract class Component {
   public abstract ConfigProvider getConfigProvider()'
}

And then a concrete class may look as:

public class ComputerComponent extends Component {
   public ConfigProvider getConfigProvider() { 
       return new ComputerConfigProvider(this);
   }
}




2. Solution similar to the first one, but have a single ConfigProvider class, with methods of adding configuration components on it.
For example, let's say a ConfigurationProvider merely holds a map of key and value.
Your code in this case can look as:

public class ConfigProvider {

    private Map<String,String> internalMap = new HashMap<String,String>();
    public String getConfigProvider() {
      //Return a string representation of the internal map
    }

    public ConfigProvider addConfigElement(String key,String value) {
        map.put(key,value); 
        return this;
    }
}

public class ComputerComponent extends Component {
    private int memory;
    private String cpu;
    public ConfigProvider getConfigProvider() {
       ConfigProvider configProvider = new ConfigProvider();
       return configProvider.addConfigElement("memory",Integer.toString(memory)).
       addConfigElement("cpu",cpu);
    } }




3. If you still want to use singletone
- Since your configuration depends on the component state, having a config provider singletone is wrong, unless you turn the getConfigProvider to be a "stateless" method -

String getConfigString(Component component)

But in this case you may consider not using a singletone, but simply having a static method, and the code will look like:

public class ConfigurationProvider {
   public static String getConfigString(Component component) {
     StringBuilder sb = new StringBuilder();
     //Append the component values to the string builder
     return sb.toString();
   }
}
Yair Zaslavsky
  • 4,091
  • 4
  • 20
  • 27