1
\$\begingroup\$

I have C# code that makes some calculations for physical models. I have this project class:

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

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

    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. E.g.

public class CrossSectionComponent: GenericComponent
{
    public CrossSectionComponent(ValueService valueService): base(valueService)
    {

    }

    ICrossSectionViewModel crossSection;

    double valuex; 

    override
    public bool Valid()
    {
        return valuex > valueService.GetValue<double>("valuey") ? true : false;
    }
}

Each component has a list of values with keys that are unique over the entire model. 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.

    private string projectUUID;
    private List<ValueViewModel> values = new List<ValueViewModel>();

    public ValueService(string projectUUID)
    {
        this.projectUUID = projectUUID;
    }

    public void RegisterValue(string key)
    {
        if (!this.hasKey(key))
        {
            values.Add(new ValueViewModel()
            {
                Key = key,
            });
        }
    }

    public void SetValue(string key, object value)
    {
        if (this.hasKey(key))
            this.values.Find(t => t.Key == key).Value = value;
    }

    public T GetValue<T>(string key)
    {
        return (T)values.Find(t => t.Key == key)?.Value;
    }

    private bool hasKey(string key)
    {
        return values.Any(t => t.Key == key);
    }
}

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.

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.Valid(component));
    }

The validity of one component does depend in most cases on values of other components. If one wants to check the validity of a components, the use sends a list of values (ValueViewModel[] data), the server creates the project where all available values are stored. Then IF AND ONLY IF all required values are set, the validity can actually be calculated. If not all values are provided inside the "data list", then the validity of said component is false. So there are two cases where component.Valid() => false

  1. The calculations yield false
  2. Not all values have been submitted

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?
\$\endgroup\$
  • 1
    \$\begingroup\$ There is alot that can be reviewed here. One tip I could give you of the bat is to use another type of collection for key-value lookup. IDictionary<string, ValueViewModel> for instance. \$\endgroup\$ – dfhwze Jun 7 at 8:38
  • \$\begingroup\$ So the validity of a component can depend on other components? And the server only stores a collection of components, but the actual values must all be provided by the client? \$\endgroup\$ – Pieter Witvoet Jun 7 at 9:24
  • \$\begingroup\$ @PieterWitvoet I updated the OP. Basically yes, they highly depend on other components, whereas the other components "only register values", the actual storage is inside the valueservice (at the moment). The server itself doesn't store anything, it gets all values via the HTTPBody of the request. \$\endgroup\$ – rst Jun 7 at 9:31
  • \$\begingroup\$ @rst Perhaps you should provide us a more tangible use case. With what you provide in the example, I would argue the need for the components altogether. \$\endgroup\$ – dfhwze Jun 7 at 9:42
  • \$\begingroup\$ The logic, how a validation is performed is stored inside the components, delivered by the component factory depending on the norm you provide. That structure is bascially set, I just don't know how to nicely share values among those components (now done by ValueService) \$\endgroup\$ – rst Jun 7 at 9:58
2
\$\begingroup\$

You have built a system of components and shared objects, but don't use its potential. In fact, for your use case the complete setup is an overkill.

  • There is no component hierarchy
  • There are no component-sensitive or -specific values
  • You only need a single component to verify any incoming request
 public IActionResult postData(
        [FromRoute] string norm, 
        [FromRoute] string component, 
        [FromBody] ValueViewModel[] data)
    {
        Project project = new Project(norm);
        project.SetValues(data);
        return Ok(project.Valid(component));
    }

For the aforementioned reasons, I don't see a need for having a list of components and values. I would rewrite the API to be as simple as possible:

  • Let the factory retrieved by FactoryGenerator get the requested component
  • Let that component validate the input values

code:

public IActionResult postData(
        [FromRoute] string norm, 
        [FromRoute] string component, 
        [FromBody] ValueViewModel[] data)
    {
        var component = FactoryGenerator.GetFactory(norm).GetComponent(component);
        return Ok(component.Valid(data));
    }
\$\endgroup\$
  • 1
    \$\begingroup\$ I have omitted a part of the code which I thought is not essential for the "problem" I wanted to address. However, I believe now it indeed is essential. I would like to rethink on how to properly ask the question and would simply accept your answer for the sake of reputation. \$\endgroup\$ – rst Jun 7 at 11:41
  • \$\begingroup\$ @rst Good call. I am happy to review an updated question that shows the potential of your API. \$\endgroup\$ – dfhwze Jun 7 at 11:42

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.