upb-lea / gym-electric-motor

Gym Electric Motor (GEM): An OpenAI Gym Environment for Electric Motors
https://upb-lea.github.io/gym-electric-motor/
MIT License
295 stars 66 forks source link

Unify arguments "converter" and "subconverters" #28

Open wkirgsn opened 4 years ago

wkirgsn commented 4 years ago

in case of multiple converters, both arguments "converter" and "subconverters" have to be set. It would be more user friendly to just go with one argument "converter", which can be either a string or a list of strings, in case of multiple converters.

atra94 commented 4 years ago

I agree with you, that it would be more user friendly to reduce it to one argument. But at the moment, I only see one simple solution to this problem.

You could just write an if-else clause in the SCMLSystem init-method, which checks the type of the converter-argument. But this would introduce a dependency between the SCMLSystem and a special converter class that I would avoid, because it would complicate the use of a different implementation that merges two converters.

Furthermore we would end up writing spaghetti-code, if we introduce such dependencies everywhere.

If such requirements of using multiple different converters will appear moreoften in the future, a redesign of the SCMLSystem to be able to handle multipleConverters inherently might be useful, because it was designed for the DC and Synchronous Motors. This solution would require much more effort and should consider a bunch of future requirements.

So finally, I would appreciate an elegant solution that handles this issue, but I would avoid additional dependencies between classes.

wallscheid commented 4 years ago

Suitable follow-up after #48 is implemented.