Should I care that passing in a class representation of an XML settings file violates the law of demeter?

Question

I'm using a tool to automatically generate a class representation of a hierarchically organized XML file. The XML file is a settings file my app need to be able to access (read-only).

If I pass in the top-level node (e.g., AppSettings) to a class that needs to access one or more settings, I can easily end up with code that looks something like this:

var windowSize = AppSettings.Views.Windows.Dashboard.Size;

This seems to be a serious violation of the Law of Demeter, but I'm wondering if I should care. I could take great pains to only pass in the exact settings I need for each class, but I'm having trouble seeing how those multiple dots are going to hurt me in this case.

Is tightly coupling my code to my XML file format likely to create maintenance issues or other problems in the future, or is this an example where it makes sense to not religiously follow an OOP design principle?

Answer 1

If you're spitting out dumb data then there's not really any better way to do it.

I would tend to try to work toward a solution where you could push and pop contexts, though.

PushContext(AppSettings)
  // do child contexts
  PushContext(Views)
    // more child contexts
    PushContext(Windows)
    // etc.
    PopContext()
  PopContext()
PopContext()

Normally the different Pushes would be in different functions or files, but are shown here for the sake of illustration. Regardless, if you push into the Views context then you just parse that as if you're at the root of the object.

If this is DumbData, though, you could also just pass the type of thing that 'Views' represents to the code that parses it. Top level, your code would look like:

views.ParseSettings(AppSettings.Views);
locale.ParseSettings(AppSettings.Locale);
network.ParseSettings(AppSettings.Network);

This would certainly be "cleaner" from an LOD POV, but it may not be worth it for the number of settings that you have. However, with the scope depth maybe the implication is that you have a lot of settings, so splitting them up into areas of responsibility (for loading and saving the settings) is probably sensible.

Answer 2

Automatic generations may be a problem for big projects.

If you will use the generated code from a single place (e.g. a single package), maybe there's no problem.

If you will make use of the code:

var windowSize = AppSettings.Views.Windows.Dashboard.Size;

in many places, you may want to hide some of this coupling making a method on the AppSettings:

getSize() {
  return Views.Windows.Dashboard.Size;
}

But if you'll need to do this to all classes, maybe it's not viable.

The best decision depends on the size of your project (and if it intends to grow), the time you have to do it, and the amount of generated code.

Answer 3

Yes, you should care, for a very pragmatic reason!

The classes where you want to use your settings absolutely don't need to be dependent on the way those settings are stored.

Imagine in the future you want to support multiple themes for your application. You will end up with not one, but many possibilities for your dashboard size, for example:

AppSettings.Views.ThemeA.Windows.Dashboard.Size;
AppSettings.Views.ThemeB.Windows.Dashboard.Size;

Your UI class still only needs one thing, a value for its variable windowSize, it doesn't need to know which theme is currently used.

It's true wherever you have an XML interface, you don't want to be dependent on the schema everywhere in your code but only in one central place.

For example you could put the settings in a Map to be used internally, like this:

public class SettingsReader {

    public static final String VIEW_WINDOW_DASHBOARD_SIZE = "Views.Windows.Dashboard.Size";

    private Map settings = new Hashmap();

    public SettingsReader(AppSettings appSettings) {
        settings.put(VIEW_WINDOW_DASHBOARD_SIZE, appSettings.Views.Windows.Dashboard.Size);
    }

    public String getSettingValue(String key) {
        return settings.get(key);
    }
}

Then you just have one place to refactor to support a theme, like this:

public class SettingsReader {

    public static final String VIEW_WINDOW_DASHBOARD_SIZE = "Views.Windows.Dashboard.Size";

    private Map settings = new Hashmap();

    public SettingsReader(AppSettings appSettings, String theme) {
        settings.put(VIEW_WINDOW_DASHBOARD_SIZE, appSettings.Views + theme + Windows.Dashboard.Size);
    }

    public String getSettingValue(String key) {
        return settings.get(key);
    }
}

A final note, just because my mix of pseudo code and java code may confuse people, especially the appSettings.Views + theme + Windows.Dashboard.Size: when working with an XML interface, xPath is usually very useful, even when working with objects thanks to the nice library JXPath (for java, I don't know for other languages).

Answer 4

All things are relative, it really depends on the size of the project and if you care about maintenance.

If you do care about maintenance then you don't want to force any restrictions imposed by your configuration source on the rest of your code base.

The best way to achieve this is to code to interfaces and hide your implementation behind that. That way your code has a contract with your configuration interface and doesn't care how the actual configuration is loaded.

public interface IConfiguration
{
    Size ViewSize { get; }
}

public class AppSettingsConfiguration : IConfiguration
{
     public Size ViewSize
     {
          return AppSettings.Views.Windows.Dashboard.Size;
     }
}

All consuming code should then be coded against the IConfiguration interface. This means you can change the way you retrieve your configuration with minimal impact.