Using abstraction and dependency injection, what if implementation-specific details need to be configurable in the UI?

Question

I have an application that loads a list of client/matter numbers from an input file and displays them in a UI. These numbers are simple zero-padded numerical strings, like "02240/00106". Here is the ClientMatter class:

public class ClientMatter
{
    public string ClientNumber { get; set; }
    public string MatterNumber { get; set; }
}

I'm using MVVM, and it uses dependency injection with the composition root contained in the UI. There is an IMatterListLoader service interface where implementations represent mechanisms for loading the lists from different file types. For simplicity, let's say that only one implementation is used with the application, i.e. the application doesn't support more than one file type at present.

public interface IMatterListLoader
{
    IReadOnlyCollection<string> MatterListFileExtensions { get; }
    IReadOnlyCollection<ClientMatter> Load(FileInfo fromFile);
}

Let's say in my initial version, I've chosen an MS Excel implementation to load the list of matters, like this:

I'd like to allow the user to configure at runtime the row and column numbers where the list starts, so the view might look like this:

And here's the MS Excel implementation of IMatterListLoader:

public sealed class ExcelMatterListLoader : IMatterListLoader
{
    public uint StartRowNum { get; set; }
    public uint StartColNum { get; set; }
    public IReadOnlyCollection<string> MatterListFileExtensions { get; set; }

    public IReadOnlyCollection<ClientMatter> Load(FileInfo fromFile)
    {
        // load using StartRowNum and StartColNum
    }
}

The row and column numbers are an implementation detail specific to MS Excel implementations, and the view model doesn't know about it. Nevertheless, MVVM dictates that I control view properties in the view model, so if I were to do that, it would be like this:

public sealed class MainViewModel
{
    public string InputFilePath { get; set; }

    // These two properties really don't belong
    // here because they're implementation details
    // specific to an MS Excel implementation of IMatterListLoader.
    public uint StartRowNum { get; set; }
    public uint StartColNum { get; set; }

    public ICommandExecutor LoadClientMatterListCommand { get; }

    public MainViewModel(IMatterListLoader matterListLoader)
    {
        // blah blah
    }
}

Just for comparison, here's an ASCII text file based implementation that I might consider for the next version of the application:

public sealed class TextFileMatterListLoader : IMatterListLoader
{
    public bool HasHeaderLine { get; set; }
    public IReadOnlyCollection<string> MatterListFileExtensions { get; set; }

    public IReadOnlyCollection<ClientMatter> Load(FileInfo fromFile)
    {
        // load tab-delimited client/matters from each line
        // optionally skipping the header line.
    }
}

Now I don't have the row and column numbers that the MS Excel implementation needed, but I have a Boolean flag indicating whether the client/matter numbers start on the first row (i.e. no header row) or start on the second row (i.e. with a header row).

I believe the view model should be unaware of the change between implementations of IMatterListLoader. How do I let the view model do its job controlling presentation concerns, but still keep certain implementation details unknown to it?

---

Here's the dependency diagram:

Roughhouse

The view need not know anything that the viewmodel does not want it to know. The view model will expose behavior that it wants the view to interact with and pass the necessary info on to its (this viewmodel) dependencies. – Bornholm

Even the view model doesn't know about the implementation detail. That's my whole point. – Roughhouse

@Roughhouse I think you are confusing between presentation vs implementation concerns. If your actor(user) do not care about start row/column details then it is not your presentation's concern. However, in your case it seems your actor is interested in start row/column details which makes it a concern of presentation as well even-though it might seem like implementation only concern. If your actor is not interested in start row/column details then, to me, it makes perfect sense to remove if from vm. – Geniculate

Why do you want StartRowNum and StartColNum in the vm? Perhaps if we understood the reasoning behind that, we'd have a path forward. – Anatomical

This is a completely made up example. The start row num and start column num are examples of configurations that the user can make, but only because the specific implementation of the IMatterListLoader has such a notion as "rows" and "columns". Another implementation might not have that; it might be an ASCII text file with comma-delimited list on a single line, for example. In such a case, there's no notion of columns and rows so the configuration available to the user would be completely different. The VM wouldn't (shouldn't) change at all though. – Roughhouse

How would you expect to manipulate different data with the same ViewModel? If you are dealing with different shapes of data, you need different ViewModels. If your user is interested in manipulating particular data, they should be exposed via the UI, which will entail exposing them from the ViewModel. Which implies: if you are exposing and manipulating different data, you might well be needing both different ViewModels and different Views, or you need a View which will present differently depending upon the shape of the ViewModel data … lots if if(this) show/hide in your view – Hunterhunting

and that will become messy and unwieldy; something I recommend highly against. Why not have a VIew ViewModel 1:1 relationship, it's easier to maintain and understand – Hunterhunting

Okay, I've made some edits to my question that I hope will bring clarity to it. Please check it over again. – Roughhouse

Also check out my updated answer. – Patriciapatrician

Answer 1

You'd need a seperate viewmodel for each type of file you intend to load.

Each viewmodel does the setup for its particular loader.

These viewmodels can then be passed in as dependencies to the main viewmodel, which calls load on each viewmodel when needed;

public interface ILoaderViewModel
{
    IReadOnlyCollection<ClientMatter> Load();
}

public class ExcelMatterListLoaderViewModel : ILoaderViewModel
{
    private readonly ExcelMatterListLoader loader;

    public string InputFilePath { get; set; }

    public uint StartRowNum { get; set; }

    public uint StartColNum { get; set; }

    public ExcelMatterListLoaderViewModel(ExcelMatterListLoader loader)
    {
        this.loader = loader;
    }

    IReadOnlyCollection<ClientMatter> Load()
    {
        // Stuff

        loader.Load(fromFile);
    }
}

public sealed class MainViewModel
{
    private ExcelMatterListLoaderViewModel matterListLoaderViewModel;

    public ObservableCollection<ClientMatter> ClientMatters
        = new ObservableCollection<ClientMatter>();

    public MainViewModel(ExcelMatterListLoaderViewModel matterListLoaderViewModel)
    {
        this.matterListLoaderViewModel = matterListLoaderViewModel;
    }

    public void LoadCommand()
    {
        var clientMatters = matterListLoaderViewModel.Load();

        foreach (var matter in clientMatters)
        {
            ClientMatters.Add(matter)
        }
    }
}

As you add more types to the application, you'd create new view models and add those as dependencies.

Repugnant

This really shines when you use the implicit DataTemplate pattern in the XAML to load the correct view for the type automatically. – Allred

I really like this solution, thank you. I realize now that the loader implementation details are at least partially a concern of the presentation logic and I'm more comfortable with that now. Your solution elegantly encapsulates and separates the concerns of the concrete loader's presentation logic. – Roughhouse

Answer 2

You could have a function that constructs the UI elements based on the specific type of the interface.

public static void ConstructUI(IMatterListLoader loader) {
    Type loaderType = loader.GetType();
    // Do logic based on type
}

You could have classes for each of the IMatterListLoader implementations, which contains logic concerning the presentation. (You don't want to mix the UI presentation logic in with the IMatterListLoader implementations).

Based on the type of the loader, you use the correct class to generate the UI elements.

Bolection

Answer 3

I would add a Draw() method to the IMatterListLoader interface. Your MainViewModel would then just call Draw() and the the actual IMatterListLoader will add whatever parameters are needed to the UI.

This is a bit conceptual as I'm not too familiar with WPF, so you might need to change the code to use UserControl's or something but the logic is the same.

For example, lets say you have AsciiMatterListLoader which requires no input from the client, then nothing will be shown in the MainViewModel. But if the ExcelMatterListLoader is loaded, the MainViewModel should add the necessary user inputs.

public sealed class AsciiMatterListLoader : IMatterListLoader
{
    public IReadOnlyCollection<string> MatterListFileExtensions { get; set; }

    public IReadOnlyCollection<ClientMatter> Load(FileInfo fromFile)
    {
        // load data with no parameters
    }

    public Panel Draw()
    {
        // Nothing needs to be drawn
        return null;
    }
}

public sealed class ExcelMatterListLoader : IMatterListLoader
{
    public uint StartRowNum { get; set; }
    public uint StartColNum { get; set; }
    public IReadOnlyCollection<string> MatterListFileExtensions { get; set; }

    public IReadOnlyCollection<ClientMatter> Load(FileInfo fromFile)
    {
        // load using StartRowNum and StartColNum
    }

    public Panel Draw()
    {
        Panel panelForUserParams = new Panel();
        panelForUserParams.Height = 400;
        panelForUserParams.Width = 200;
        TextBox startRowTextBox = new TextBox();
        startRowTextBox.Name = "startRowTextBox";
        TextBox startColumnTextBox = new TextBox();
        startColumnTextBox.Name = "startColumnTextBox";
        panelForUserParams.Children().Add(startRowTextBox);
        panelForUserParams.Children().Add(startColumnTextBox);
        return panelForUserParams;
    }
}

public sealed class MainViewModel
{
    public string InputFilePath { get; set; }
    public ICommandExecutor LoadClientMatterListCommand { get; }

    public MainViewModel(IMatterListLoader matterListLoader)
    {
        var panel = matterListLoader.Draw();
        if (panel != null)
        {
                // Your MainViewModel should have a dummy empty panel called "placeHolderPanelForChildPanel"
                var parent = this.placeHolderPanelForChildPanel.Parent;
                parent.Children.Remove(this.placeHolderPanelForChildPanel); // Remove the dummy panel
                parent.Children.Add(panel); // Replace with new panel
        }
    }
}

You might need to use event handlers to pass the user input changes to the IMatterListLoader or maybe make IMatterListLoader a UserControl.

Edit

@rory.ap is right, the service layer shouldn't know about the UI components. Here is my adjusted answer where the IMatterListLoader just exposes the properties it needs by using a dictionary as a PropertyBag instead of telling the UI what to draw. This way the UI layer does all the UI work:

public interface IMatterListLoader
{
    IReadOnlyCollection<ClientMatter> Load(FileInfo fromFile);
    IDictionary<string, object> Properties { get; }
    void SetProperties(IDictionary<string, object> properties);
}

public sealed class AsciiMatterListLoader : IMatterListLoader
{
    public IReadOnlyCollection<string> MatterListFileExtensions { get; set; }

    public IDictionary<string, object> Properties
    {
        get 
        {
            return new Dictionary<string, object>(); // Don't need any parameters for ascii files
        }
    }

    public void SetProperties(IDictionary<string, object> properties)
    {
        // Nothing to do
    }

    public IReadOnlyCollection<ClientMatter> Load(FileInfo fromFile)
    {
        // Load without using any additional params
        return null;
    }
}

public sealed class ExcelMatterListLoader : IMatterListLoader
{
    private const string StartRowNumParam = "StartRowNum";
    private const string StartColNumParam = "StartColNum";

    public uint StartRowNum { get; set; }
    public uint StartColNum { get; set; }
    public IReadOnlyCollection<string> MatterListFileExtensions { get; set; }

    private bool havePropertiesBeenSet = false;

    public IDictionary<string, object> Properties
    {
        get
        {
            var properties = new Dictionary<string, object>();
            properties.Add(StartRowNumParam, (uint)0); // Give default UINT value so UI knows what type this property is
            properties.Add(StartColNumParam, (uint)0); // Give default UINT value so UI knows what type this property is

            return properties;
        }
    }

    public void SetProperties(IDictionary<string, object> properties)
    {
        if (properties != null)
        {
            foreach(var property in properties)
            {
                switch(property.Key)
                {
                    case StartRowNumParam:
                        this.StartRowNum = (uint)property.Value;
                        break;
                    case StartColNumParam:
                        this.StartColNum = (uint)property.Value;
                        break;
                    default:
                        break;
                }
            }

            this.havePropertiesBeenSet = true;
        }
        else
            throw new ArgumentNullException("properties");
    }

    public IReadOnlyCollection<ClientMatter> Load(FileInfo fromFile)
    {
        if (this.havePropertiesBeenSet)
        {
            // Load using StartRowNum and StartColNum
            return null;
        }
        else
            throw new Exception("Must call SetProperties() before calling Load()");
    }
}

public sealed class MainViewModel
{
    public string InputFilePath { get; set; }
    public ICommandExecutor LoadClientMatterListCommand { get; }
    private IMatterListLoader matterListLoader;

    public MainViewModel(IMatterListLoader matterListLoader)
    {
        this.matterListLoader = matterListLoader;

        if (matterListLoader != null && matterListLoader.Properties != null)
        {
            foreach(var prop in matterListLoader.Properties)
            {
                if (typeof(prop.Value) == typeof(DateTime))
                {
                    // Draw DateTime picker for datetime value
                    this.placeHolderPanelForParams.Add(new DateTimePicker() { Name = prop.Key });
                }
                else 
                {
                    // Draw textbox for everything else
                    this.placeHolderPanelForParams.Add(new TextBox() { Name = prop.Key });

                    // You can also add validations to the input here (E.g. Dont allow negative numbers of prop is unsigned)
                    // ...
                }
            }
        }
    }

    public void LoadFileButtonClick(object sender, EventArgs e)
    {
        //Get input params from UI
        Dictionary<string, object> properties = new Dictionary<string, object>();
        foreach(Control propertyControl in this.placeHolderPanelForParams().Children())
        {
            if (propertyControl is TextBox)
                properties.Add(propertyControl.Name, ((TextBox)propertyControl).Text);
            else if (propertyControl is DateTimePicker)
                properties.Add(propertyControl.Name, ((DateTimePicker)propertyControl).Value);
        }

        this.matterListLoader.SetProperties(properties);
        this.matterListLoader.Load(null); //Ready to load
    }
}

Patriciapatrician

But IMatterListLoader doesn't have any concept of "Draw" because its a service class, not a presentation class. – Roughhouse

@Roughhouse You're right, I adjusted my answer for a better design. – Patriciapatrician

Answer 4

Not sure why nobody suggested property attributes and reflection

Just create a new Attribute, eg:

[AttributeUsage(AttributeTargets.Property, AllowMultiple = false)]
public class ExposeToViewAttribute : Attribute
{
    public string Name { get; set; }

    public ExposeToViewAttribute([System.Runtime.CompilerServices.CallerMemberName]string name = "")
    {
        this.Name = name;
    }
}

and make sure it gets added in your view

var t = matterListLoader.GetType();
var props = t.GetProperties().Where((p) => p.GetCustomAttributes(typeof(ExposeToViewAttribute), false).Any());
foreach(var prop in props)
{
    var att = prop.GetCustomAttributes(typeof(ExposeToViewAttribute), true).First() as ExposeToViewAttribute;
    //Add to view
}

approach will not get much cleaner.

Usage then would be as simple as:

[ExposeToView]
public int Something { get; set; }

[ExposeToView("some name")]
public int OtherFieldWithCustomNameThen { get; set; }

If you use something like WPF however, there are other solutions that may work better for you.

Levitical

It's always best to avoid reflection where possible, for the reasons in this relativly short article: brooknovak.wordpress.com/2013/09/24/… – Hypoderma

Reflection is a Tool like many other. This here is even less "hostile" then what is described in there and a perfect example where one could use attributes to make coding simpler – Levitical

Reflection isn't a tool like any other - other tools don't break encapsulation in the way reflection does. – Hypoderma

so what? this case is even described as "when to use" in your linked article Dependency injection frameworks ... now guess what this does? it reads the attributes on public properties and uses them. This is as "hostile" and "bad" as XmlElementAttribute & co are. Just because you got teached at some point in time "avoid reflection" does not means that you should never use language methods (and attributes ARE language methods!). The difference to a framework like public IEnumerable<Property> Properties is that this is not as invasive and keeps code clean – Levitical

I "got learned" that using reflection when there's alternatives causes issues, while some still prefer it, this isn't a place I'd use it. – Hypoderma