Added abstraction for power value sources. #1439
Conversation
staudtMarius
added
enhancement
# Conflicts: # src/main/java/edu/ie3/datamodel/io/factory/timeseries/LoadProfileFactory.java # src/main/java/edu/ie3/datamodel/io/source/LoadProfileSource.java # src/main/java/edu/ie3/datamodel/io/source/csv/CsvLoadProfileSource.java # src/main/java/edu/ie3/datamodel/io/source/sql/SqlLoadProfileSource.java # src/main/java/edu/ie3/datamodel/models/timeseries/repetitive/LoadProfileTimeSeries.java # src/main/java/edu/ie3/datamodel/models/timeseries/repetitive/RandomLoadProfileTimeSeries.java # src/test/groovy/edu/ie3/datamodel/io/source/sql/SqlLoadProfileSourceIT.groovy
| * Interface for the input data of {@link #getValue(InputData)}. The data is used to determine the | ||
| * next power. | ||
| */ | ||
| sealed interface InputData permits PowerValueSource.TimeSeriesInputValue { |
There was a problem hiding this comment.
Maybe we can think about a different name for the interface here. InputData seems quite generic. Spontaneously I'd think of something that includes Identifier, maybe PowerValueIdentifier, or something better?
|
|
||
| /** Interface defining base functionality for power value sources. */ | ||
| public sealed interface PowerValueSource< | ||
| P extends PowerProfile, ID extends PowerValueSource.InputData> |
There was a problem hiding this comment.
Please check out the sonarqube issue
| @Override | ||
| public int hashCode() { | ||
| return Objects.hash(super.hashCode(), profile); | ||
| return Objects.hash(profile); |
There was a problem hiding this comment.
Wouldn't do this, especially because the equals method still relies on the super class
| /** | ||
| * Method to get the next power value based on the provided input data. | ||
| * | ||
| * @param data input data that is used to calculate the next power value. | ||
| * @return an option for the power value. | ||
| */ | ||
| Optional<PValue> getValue(ID data); |
There was a problem hiding this comment.
I think this method now becomes obsolete with the addition of getValueSupplier. We can just use a supplier all the time, even if it always returns the same value.
| @Override | ||
| public P getProfile() { | ||
| return loadProfileTimeSeries.getLoadProfile(); | ||
| } |
There was a problem hiding this comment.
Unnecessary ovveride imho
| if (loadValueOption.isPresent()) { | ||
| LoadValues<P> loadValue = loadValueOption.get(); | ||
| return () -> Optional.of(loadValue.getValue(time, profile)); | ||
| } else { | ||
| return Optional::empty; | ||
| } |
There was a problem hiding this comment.
Maybe you can use the map function on the optional here?
| /** | ||
| * Get all {@link ZonedDateTime}s after the given time. | ||
| * | ||
| * @param time given time | ||
| * @return a list of all time keys | ||
| */ | ||
| public abstract List<ZonedDateTime> getTimeKeysAfter(ZonedDateTime time); |
There was a problem hiding this comment.
What do you mean with finally? I think we added this method not long ago. I just moved this method to another place.
There was a problem hiding this comment.
Thought we would get rid of this method...
| /** | ||
| * Get all {@link ZonedDateTime}s after the given time. | ||
| * | ||
| * @param time given time | ||
| * @return a list of all time keys | ||
| */ | ||
| public List<ZonedDateTime> getTimeKeysAfter(ZonedDateTime time) { | ||
| return timeToValue.keySet().stream().filter(timeKey -> timeKey.isAfter(time)).sorted().toList(); | ||
| } |
There was a problem hiding this comment.
Should me mark this method as obsolete, or just remove it completely? Or are there reasons to keep it?
There was a problem hiding this comment.
I think that we are using this method in SIMONA. But I will check this later and remove it, if we don't use it.
There was a problem hiding this comment.
This would be replaced by getNextDateTime in SIMONA I think. But we don't have to delete this method right away.
Resolves #1438
Merge #1450 first.