-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor ThermalEnergyDemand #918
base: dev
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pushing this further, @marvinheintze! Also a good idea to add the cases for negative demands. I thought a bit on this and I guess it will be easier to go just with positive demands as we at the moment doesn't have any specific use case that requires negative demands. So please remove them again, I'm sorry for the confusion that may arised.
if ( | ||
math.abs(possible.toKilowattHours) < math.abs( | ||
required.toKilowattHours | ||
) && math.signum(possible.toKilowattHours) <= math.signum( | ||
required.toKilowattHours | ||
) | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If possible would be -2 and required 1 this would not throw an exception, or am I wrong?
Lets go with just positive values (at least for now). So it would be great to check first if req or pos is negative plus if possible is smaller than required. In any case we should throw an an InvalidParameterException with a suitable message.
resolves #917
I started here, but more needs to be done. Feel free to take over :)