I am currently planning a monitoring server for a distributed system. In a file (or maybe a database, someday) I save all the servers and parameters that I want to monitor. When the monitoring system starts, I parse that configuration file and store the content in a Configuration class (see source code below). In order to be independent from the storage type (file, database, …) I use the IConfigurationHandler Interface (ok, I admit that the name doesn’t really fit :)), whose implementations are responsible for reading/writing the file content.
I considered the following design:
public class Configuration
{
private IConfigurationHandler _configHandler;
public Configuration(IConfigurationHandler configHandler)
{
this._configHandler = configHandler;
this.Load();
}
public void Load()
{
this._configHandler.LoadConfigurationInto(this);
}
}
public class XmlDocumentConfigurationHandler : IConfigurationHandler
{
public void LoadConfigurationInto(Configuration configuration)
{
configuration.X = "...";
configuration.Y = "...";
configuration.Z = "...";
}
}
If I wanted to use the Configuration class, the only thing I would have to do is add a dependency to the constructor (assuming that I am using a DI-Framework).
public class ConfigurationConsumer
{
public ConfigurationConsumer(Configuration configuration)
{
// Do something with the configuration
}
}
But somehow this code gives me a bad feeling. What do you guys think about it?
Is it “good” if an object let’s configure itself by another class?
Even though it “feels” wrong, I cannot think of a particular case where this design has any negative impact.
Another approach would be to call the IConfigurationHandler directly from the class that needs the Configuration, as follows:
public class Configuration
{
public Configuration() { }
}
public class XmlDocumentConfigurationHandler : IConfigurationHandler
{
public Configuration GetConfiguration()
{
Configuration configuration = new Configuration();
configuration.X = "...";
configuration.Y = "...";
configuration.Z = "...";
}
}
public class ConfigurationConsumer
{
public ConfigurationConsumer(IConfigurationHandler configurationHandler)
{
var configuration = configurationHandler.GetConfiguration();
// Do something with the configuration
}
}
The problem with this approach is, that I would need to accept an IConfigurationHandler, even though the only thing I need is a Configuration object.
Here is a (very) rough outline of my architecture:
And here is an example for the content of the configuration file:
<servers>
<server id="1">
<monitored_values>
<monitored_value name="CpuUsage"/>
<monitored_value name="..."/>
</monitored_values>
<!--some more configuration-->
</server>
<server id="...">
<!--some more configuration-->
</server>
<server id="n">
<!--some more configuration-->
</server>
</servers>
The actual configuration file then would, among others, contain a list with all servers, which themselves contain a list with their monitored parameters.
4
I think it gives you a “bad feeling” because it breaks encapsulation. When doing this:
configuration.X = "...";
configuration.Y = "...";
configuration.Z = "...";
what should be the internal state of a Configuration is publicly exposed.
So far I understand, something like this should be enough ?
public class Configuration
{
private X x;
private Y y;
private Z z;
public Configuration(X x, Y y, Z z)
{
this.x = x;
this.y = y;
this.z = z;
}
}
2
Is it “good” if an object let’s configure itself by another class?
Yes. It’s done all the time.
As @spotted said, it’s encapsulation. But I want to add that this encapsulation should be realized through the concept of the “factory”.
Make a factory class that has-a configuration object. Client code only tells the factory what object to make. The factory can be complex as needed to create according to the configuration “specification”. The factory concept is meant to scale and address object structure: “factory method”, “factory”, and “abstract factory” reflect this scaling. “Builder” is a companion pattern addressing object creation complexity.