teragrep / cfe_31

0 stars 0 forks source link

Refactor validation calls in objects #53

Open 51-code opened 4 months ago

51-code commented 4 months ago

Description

Related somewhat to #51.

There are many objects with a valid() function e.g. HostConfig and HostConfigEntry in main branch, many more objects coming in remove-nullpointerexceptions branch. The valid() function is supposed to be called after initialization and an Exception thrown if the function returns false. This introduces temporal coupling where the validation has to be remembered or else the software won't work as expected.

I'd recommend running validation in the object itself when calling its other functions, or rather similarly creating a composable decorator for all objects that have to validate their data. Then the decorator would validate the data, throw an exception if necessary and then call the wanted method in the object inside the decorator.

Could @kortemik or @eemhu comment on this being a necessary change at all? Perhaps the valid() function is indeed valid as is?

Temporal coupling is mentioned in Elegant objects vol 1 pages 83 and 129 and in Elegant Objects vol 2 pages 71-83.

Also validation is talked about in Elegant objects vol 2. "We should not pollute our code with checks, tests, assertions, and validations. All of these must exist outside of the core objects." (page 35). There is also an example of the kind of a decorator I suggested.

eemhu commented 3 months ago

I think it would be clearer and also less likely to accidentally not validate an object, if some sort of decorator pattern would be used instead:

public class ValidatedObject() {
  private final Object o;
  public ValidatedObject(Object o) {
    this.o = o;
  }

  public ValidatedObject get() {
    if (/* Object valid */) {
       return o;
    }
    throw new IllegalStateException("Object was not valid due to xyz");
  }
}
MoonBow-1 commented 2 months ago

This issue has been submitted for internal review

MoonBow-1 commented 2 months ago

This issue will also close #25 due to changes to new interfaces

MoonBow-1 commented 2 months ago

Also closes #34 with changes to the GenerateFile interface

MoonBow-1 commented 1 month ago

Requires more unit tests, as requested internally