Logic is stored in Components that receive input data and update output data via Observables

Question

I have C# ASP.Net Core Webapi application that makes some calculations for physical models. I have this project class:

public class Project {
    public IEnumerable<GenericComponent> components;
    private ValueService valueService = new ValueService();

    public Project(string norm) {
        components = FactoryGenerator.GetFactory(norm).GetComponents(valueService);
    }

    public IEnumerable<ValueViewModel> GetResults() {
        return valueService.GetResults();
    }

    public void SetValues(IEnumerable<ValueViewModel> values) {
        values.ToList().ForEach(t => valueService.SetValue(t.Key, t.Value));
    }

    public bool Valid(string componentName) {
        return components.Where(s => s.Title == componentName).Single().Valid();
    }
}

That creates smaller components that do some calculations. Depending on the norm parameter, another set of components is created (via the factory). Each component has a list of variables that may exist (the frontend know which ones) and it can do some calculations on those variables and on variables that are being initialized in other components (with 3 example implementations of GenericComponent).

public abstract class GenericComponent : IObserver<Tuple<string, object>> {
    public readonly IValueService valueService;
    protected IDisposable cancellation;
    public List<string> variables;
    public List<string> observingVariables;

    public GenericComponent(IValueService _valueService) {
        valueService = _valueService;
    }
    public string Title { get; set; }
    public void OnCompleted() { throw new NotImplementedException();   }
    public void OnError(Exception error) {// no implementation
    }
    public abstract void OnNext(Tuple<string, object> value); 
    public abstract bool Valid();
}

public class LoadComponent : GenericComponent {     
    public LoadComponent(IValueService valueService) : base(valueService) {
        variables = new List<string>() { "load" };
        variables.ForEach(s => valueService.RegisterValue(s));
        cancellation = valueService.Subscribe(this, variables);
    }
    public override bool Valid() => valueService.GetValue("load")?.Valid ?? false;
    public override void OnNext(Tuple<string, object> value) {
        double load = (double)value.Item2;
        if (Math.Abs(load) < 1) {
            valueService.SetWarning(value.Item1, "Small load, should be larger");
        }
    }
}


public class ValidationComponent : GenericComponent {

    public ValidationComponent(IValueService valueService) : base(valueService) {
        variables = new List<string>() { "validation.load" };
        variables.ForEach(s => valueService.RegisterValue(s));
        observingVariables = new List<string>() { "load", "area" };
        cancellation = valueService.Subscribe(this, observingVariables);
    }
    private double CalcMargin() {
        return 0.5 * (double)valueService.GetValue("area").Value 
            - (double)valueService.GetValue("load").Value;
    }
    public override bool Valid() => CalcMargin() > 0;
    public override void OnNext(Tuple<string, object> value) {
        if (valueService.GetValue("load").Value != null &&
            valueService.GetValue("area").Value != null) {
            double margin = CalcMargin();
            bool valid = margin > 0;
            valueService.SetValue("validation.load", "margin is: " + margin.ToString(), 
                valid);
            if (margin < 1) {
                valueService.SetWarning("validation.load", "Very tight margin");
            }
        }
    }
}

public class CrossSectionComponent : GenericComponent {
    public CrossSectionComponent(IValueService valueService) : base(valueService) {
        variables = new List<string>() { "area", "radius" };
        variables.ForEach(s => valueService.RegisterValue(s));
        cancellation = valueService.Subscribe(this, variables);
    }
    public override bool Valid() => valueService.GetValue("radius")?.Valid ?? false;
    public override void OnNext(Tuple<string, object> value) {
        if (value.Item1 == "radius") {
            double radius = (double)value.Item2;
            if (radius < 0) {
                valueService.SetError("radius", "Radius is must be larger than 0");
            } else if (radius < 1) {
                valueService.SetWarning("radius", "Radius is very small");
            }
            valueService.SetValid("radius", true);
            valueService.SetValue("area", radius * radius * Math.PI, true);
        }
    }
}

Each component has a list of values with keys that are unique over the entire model (here I probably need more checks, whether truly all keys are unique). At the moment I use a ValueService, where each component registers its values with the respective keys. The ValueService can be used to get values from other components in order to do validation calculations or intermediate steps.

public class ValueService : IValueService {

    private IDictionary<string, ValueViewModel> values = new Dictionary<string, ValueViewModel>();
    private IDictionary<string, List<IObserver<Tuple<string, object>>>> observers = 
        new Dictionary<string, List<IObserver<Tuple<string, object>>>>();

    public void RegisterValue(string key) {
        if (!values.ContainsKey(key)) {
            values.Add(key, new ValueViewModel() {
                Key = key,
                Value = null
            });
            observers.Add(key, new List<IObserver<Tuple<string, object>>>());
        }
    }

    public IEnumerable<ValueViewModel> GetResults() {
        return values.Where(s => s.Value.Value != null).Select(s => s.Value);
    }

    public void SetValue(string key, object value) {
        if (values.ContainsKey(key)) {
            values[key].Value = value;
            observers[key].ForEach(t => {
                t.OnNext(new Tuple<string, object>(key, value));
            });
        }
    }

    public void SetValue(string key, object value, bool status) {
        SetValue(key, value);
        SetValid(key, status);
    }

    public void SetValid(string key, bool status) {
        if (values.ContainsKey(key)) {
            values[key].Valid = status;
        }
    }

    public ValueViewModel GetValue(string key) {
        if (values.ContainsKey(key))
            return values[key];
        return null;
    }

    public IDisposable Subscribe(IObserver<Tuple<string, object>> observer) {
        return new Unsubscriber(observers.Values.ToList().SelectMany(s => s).ToList(), observer);
    }

    public IDisposable Subscribe(IObserver<Tuple<string, object>> observer, IEnumerable<string> keys) {
        keys.ToList().ForEach(key => {
            if (!observers.ContainsKey(key))
                observers.Add(key, new List<IObserver<Tuple<string, object>>>());
            if (!observers[key].Contains(observer))
                observers[key].Add(observer);
        });
        return new Unsubscriber(observers.Values.ToList().SelectMany(s => s).ToList(), observer);
    }

    public void SetWarning(string key, string warning) {
        if (values.ContainsKey(key)) {
            values[key].Warning = warning;
            values[key].Valid = true;
        }
    }

    public void SetError(string key, string error) {
        if (values.ContainsKey(key)) {
            values[key].Error = error;
            values[key].Valid = false;
        }
    }

}

The ValueViewModel is used to communicate with the front end

public class ValueViewModel
{
    public string Key { get; set; }
    public object Value { get; set; }
    public bool Valid { get; set; }
    public string Warning { get; set; }
    public string Error { get; set; }
}

This is part of a stateless API, i.e. for each HTTP request (at the moment) a new Project object is generated and used to make validations (via an array of ValueViewModels).

My API controller looks like this:

    public IActionResult postData([FromRoute] string norm, [FromRoute] string component, [FromBody] ValueViewModel[] data)
    {
        Project project = new Project(norm);
        project.SetValues(data);
        return Ok(project.GetResults()); // <<-- previously mistake, it should be GetResults, that returns a ValueViewModel with populated values (where possible)
    }

In order to make some local tests, I created the following console application

class Program {
    static void Main(string[] args) {
        Console.WriteLine("Hello world");
        Project project = new Project("Eurocode_CH");
        List<ValueViewModel> values = new List<ValueViewModel> {
            new ValueViewModel() {
                Key = "radius",
                Value = 12.0
            }
        };
        project.SetValues(values);
      Console.WriteLine(Newtonsoft.Json.JsonConvert.SerializeObject(project.GetResults()));
        values[0].Value = -0.4;
        project.SetValues(values);
      Console.WriteLine(Newtonsoft.Json.JsonConvert.SerializeObject(project.GetResults()));
        values[0].Value = 0.4;
        project.SetValues(values);
      Console.WriteLine(Newtonsoft.Json.JsonConvert.SerializeObject(project.GetResults()));
        values.Add(new ValueViewModel() {
            Key = "load",
            Value = 1.2
        });
        project.SetValues(values);
        Console.WriteLine(Newtonsoft.Json.JsonConvert.SerializeObject(project.GetResults()));
    }
}

This is the output

[{"Key":"area","Value":452.38934211693021,"Valid":true,"Warning":null,"Error":null},{"Key":"radius","Value":12.0,"Valid":true,"Warning":null,"Error":null}]

[{"Key":"area","Value":0.50265482457436694,"Valid":true,"Warning":null,"Error":null},{"Key":"radius","Value":-0.4,"Valid":true,"Warning":null,"Error":"Radius is must be larger than 0"}]

[{"Key":"area","Value":0.50265482457436694,"Valid":true,"Warning":null,"Error":null},{"Key":"radius","Value":0.4,"Valid":true,"Warning":"Radius is very small","Error":"Radius is must be larger than 0"}]

[{"Key":"area","Value":0.50265482457436694,"Valid":true,"Warning":null,"Error":null},{"Key":"radius","Value":0.4,"Valid":true,"Warning":"Radius is very small","Error":"Radius is must be larger than 0"},{"Key":"load","Value":1.2,"Valid":false,"Warning":null,"Error":null}]

My concerns are

• When the project continues, the number of components will get quite large, I fear there will be a performance issue if for each request the whole set of components needs to be generated --> should I use LazyLoading?
• Is the ValueService approach good to share data between the components? Is there a better solution?
• I still need to implement a specific value-getter, e.g. a user requests the ValueViewModel for a specific key, the components needs to yield an error or warning if not all values were set.
• I think "AbstractComponent" is a better name than "GenericComponent", on a side note.

Answer 1

Design Choice

• At first, your API looked like a convoluted pattern to validate user input. However, since you elaborated that validation is highly configurable by country, implies many backend calculations and requires the reuse of intermediate results, I feel your design is justified.
• I will focus on design decisions that I can't make for you, but I hope you take into consideration for improving your code.

---

Data Integrity

You allow consumers of the API to directly change the data, bypassing your code. Is this by design or an unwanted leak of data? If this behavior is not wanted, you should either use a different class for storing the values internally, or clone the data both in SetValues and GetValues.

List<ValueViewModel> values = new List<ValueViewModel> {
     new ValueViewModel() {
         Key = "radius",
         Value = 12.0
     }
};
values[0].Value = 0.4;       // <- change data without notifying observers!
project.SetValues(values);   // <- only now, observers get notified

When using ValueViewModel as internal type to store the values, would you allow consumers to change all these properties?

public class ValueViewModel
{
    public string Key { get; set; }     // <- change key, while having a different key in the dictionary
    public object Value { get; set; }   // <- change value, but perhaps the type is invalid
    public bool Valid { get; set; }     // <- override validity, even though there might be an error
    public string Warning { get; set; } // <- are multiple warnings possible?
    public string Error { get; set; }   // <- are multiple errors possible, can the data be valid even if there is an error?
}

You have included convenience methods which enforce some enacapsulation, but the consumer is not forced to use these, and can still bypass them by calling GetValues.

  public void SetError(string key, string error) {
       if (values.ContainsKey(key)) {
             values[key].Error = error;
             values[key].Valid = false;
       }  
  }

---

Consistency

Consumers can get the values as ValueViewModel by calling GetValues, but observers get a different representation of the data Tuple<string, object>. Your components, being observers, work on both these structures. This introduces unnecessary complexity for consumers of the API.

public IDisposable Subscribe(IObserver<Tuple<string, object>> observer) {
     return new Unsubscriber(observers.Values.ToList()
         .SelectMany(s => s).ToList(), observer);
}

---

Interface compatibility

A component implements IObserver<T>. This means it can be used in any model that works with observers. Models that call OnCompleted will receive a nice exception. Is there a specific reason to throw an error, or could you just leave it empty as you did with OnError?

public void OnCompleted() { throw new NotImplementedException(); }

---

Single Responsibility

A project cannot be instantiated with components, instead it requires a norm. A project should not care about the norm, it should care about its components. This would facilitate unit testing. Else you would have to mock FactoryGenerator, while you just need components to work with.

 public Project(string norm) {
        components = FactoryGenerator.GetFactory(norm).GetComponents(valueService);
    }

I would instead make a project factory to create a project given a norm.

public Project(IEnumerable<GenericComponent> components, ValueService valueService) {
    // perform arg checks ..
    this.components = components;
    this.valueService = valueService;
}

public static class ProjectFactory {
    public static Project Create(string norm, ValueService valueService) {
        return new Project(
            FactoryGenerator.GetFactory(norm).GetComponents(valueService)
          , valueService);
    }
}

---

Conflict Resolutions

Since any component can work with any value, you need a good specification on which components are the owner of which values. What if two components want to register a value using the same key?

• first one wins: the other uses the value registered by the first
• last one wins: the first one will use the value overriden by the second
• error: make clear there is a flaw in the design

public void RegisterValue(string key) {
        if (!values.ContainsKey(key)) {
            values.Add(key, new ValueViewModel() {
                Key = key,
                Value = null
            });
            observers.Add(key, new List<IObserver<Tuple<string, object>>>());
        }  // <- what else ??
    }

I would opt for the first choice. This way, components are independant and flexible. They will only register values that haven't already been registered by another component. There should still be a policy that given a unique key, any component uses the same type and definition for the value.

---

General Design

ValueViewModel

• Decide whether to use this type internally in your API or you use another type with better encapsulation, maybe even immutable.

ValueService

• Be consistent and use only one internal type, both usable by consumers of the API and observers of the values.
• Determine a strategy for handling conflicts when registering values.

Component

• Consider creating an interface IComponent for better testability.
• Take into account values might already be registered when registering values.
• Decide which component is responsible for cascading updates of other values when changing a value. radius changed -> change area.
• Ensure two-way cascaded updates don't result in an infinite loop. What if area changed -> update radius is allowed as well?

Project

• Extract configuration logic and move it to a factory.

---

Q&A

When the project continues, the number of components will get quite large, I fear there will be a performance issue if for each request the whole set of components needs to be generated --> should I use LazyLoading?

Since your components are passive objects (as opposed to active object) that get instantiated on demand given the norm, I don't see the need to implement lazy loading here.

Is the ValueService approach good to share data between the components? Is there a better solution?

By using ValueService you have decoupled component logic with the extended state (the value cache) of a project. This is common practice when building state machines.

I still need to implement a specific value-getter, e.g. a user requests the ValueViewModel for a specific key, the components needs to yield an error or warning if not all values were set.

I have addressed possible strategies for conflict resolution in the review. You could extend these strategies to your getter and setters as well.

I think "AbstractComponent" is a better name than "GenericComponent", on a side note.

AbstractComponent is the better name, although it's a typical java name. In C#, it's more common to call this ComponentBase. Some API's would call it ComponentSkeleton.