Refactor the Wine Engine Object #1066
Conversation
| var containerConfiguration = configFactory.open(this.prefixDirectory() + "/phoenicis.cfg"); | ||
| var architecture = containerConfiguration.readValue("wineArchitecture", "x86"); | ||
| const containerConfiguration = this.configFactory.open(this.prefixDirectory() + "/phoenicis.cfg"); | ||
| const architecture = containerConfiguration.readValue("wineArchitecture", "x86"); |
There was a problem hiding this comment.
I would like to move the "extraction" of the three value wineArchitecture, wineDistribution and wineVersion to a dedicated method that returns a json containing the three values.
What do you think?
There was a problem hiding this comment.
I'm not sure that a json is clearer compared to readValue.
There was a problem hiding this comment.
The main issue for me is the extraction code and the access to containerConfiguration which I would like to hide behind a method.
In addition it is quite easy to access the values:
const { architecture, distribution, version } = fetchContainerConfiguration(this.prefixDirectory() + "/phoenicis.cfg");There was a problem hiding this comment.
In my opinion, the architecture method already encapsulates this.
There was a problem hiding this comment.
I don't understand? The architecture() method only returns a single value, while I suggest to return three wine related values at once.
There was a problem hiding this comment.
I know. However, your point is that you want to hide something in a method. In my opinion the architecture() method already does so. Therefore I do not see the need for additional encapsulation.
There was a problem hiding this comment.
Let me demonstrate what I want to do in a different (new) PR. This is out of scope anyway.
|
Additionally we should really add some more comments to this class! |
|
About the Codacy error: I don't want to fix the issue because it seems suspect to me |
|
You're right about the Codacy error. |
- refactor the wine engine object - add some more error messages
This PR refactors the Wine engine object with newer JS language features like
classand streams