SERG-Delft / andy

Andy assesses student's test code. It's used in CSE1110, TU Delft.
MIT License
78 stars 23 forks source link

Resolve "Create a proper builder for Context" #239

Closed St1p42 closed 1 year ago

St1p42 commented 1 year ago

Closes #191

Builder

The Context class now has a builder ContextBuilder with setters and build() method. Setters not used during the construction of Context in the existing code are not present there, so if there are more cases where Context is created - these setters will have to be added.

builder

Director

Instead of calling builder.setA().setB().setC().build() to get a context I decided to have still some premade methods that are now in the class ContextDirector. Inside these construction methods, I also return the Context object to avoid using builder from the client code directly. I replaced all of the occurrences of previous initialization with these construction methods, though I am not sure if a director is necessary (I guess builder.setA().setB().setC().build() also works fine). Currently, it has constructBase (accepting action + directory config as parameters), constructWithLibraries (accepting action + directory config + libraries list), and constructWithClassLoader (accepting action + directory config + libraries list + class loader).

Other changes

Unnecessary setters in the Context class are now removed, though the ones which are used outside of initialization (for example when the setter is applied after passing initialized context to another method) are still there. I also created some tests for Directors' construction methods just in case. Besides a folder Context was created to put there Context class + builder + director. So if there are too many file changes in this pull request, it is because of the fact that all imports of Context in the project were changed (since now it has a different path).

mauricioaniche commented 1 year ago

I have to review this one on the computer!! I'm on vacation right now, so I'll do it next week!!

Thanks for the PR!