-
Notifications
You must be signed in to change notification settings - Fork 7
Introducing SqlIndividualTimeSeriesMetaInformation & refactorings #518
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
Changes from all commits
b6b6600
c14b34a
507f041
db10433
f833d6c
6b55889
aee87b2
e834d34
48ef593
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,16 +3,16 @@ | |
| * Institute of Energy Systems, Energy Efficiency and Energy Economics, | ||
| * Research group Distribution grid planning and operation | ||
| */ | ||
| package edu.ie3.datamodel.io.csv; | ||
| package edu.ie3.datamodel.io.naming; | ||
|
|
||
| import java.util.Objects; | ||
| import java.util.UUID; | ||
|
|
||
| /** Meta information, that can be derived from a certain file name */ | ||
| public abstract class FileNameMetaInformation { | ||
| /** Meta information, that describe a certain data source */ | ||
| public abstract class DataSourceMetaInformation { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not fully convinced with the name... Yes, it does describe a data source, but it does not describe a data source in general. It's more describing a time series data source. But There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In that case, how about simply There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| private final UUID uuid; | ||
|
|
||
| protected FileNameMetaInformation(UUID uuid) { | ||
| protected DataSourceMetaInformation(UUID uuid) { | ||
| this.uuid = uuid; | ||
| } | ||
|
|
||
|
|
@@ -23,7 +23,7 @@ public UUID getUuid() { | |
| @Override | ||
| public boolean equals(Object o) { | ||
| if (this == o) return true; | ||
| if (!(o instanceof FileNameMetaInformation that)) return false; | ||
| if (!(o instanceof DataSourceMetaInformation that)) return false; | ||
| return uuid.equals(that.uuid); | ||
| } | ||
|
|
||
|
|
@@ -34,6 +34,6 @@ public int hashCode() { | |
|
|
||
| @Override | ||
| public String toString() { | ||
| return "FileNameMetaInformation{" + "uuid=" + uuid + '}'; | ||
| return "DataSourceMetaInformation{" + "uuid=" + uuid + '}'; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,14 +3,14 @@ | |
| * Institute of Energy Systems, Energy Efficiency and Energy Economics, | ||
| * Research group Distribution grid planning and operation | ||
| */ | ||
| package edu.ie3.datamodel.io.csv.timeseries; | ||
| package edu.ie3.datamodel.io.naming.timeseries; | ||
|
|
||
| import edu.ie3.datamodel.io.csv.FileNameMetaInformation; | ||
| import edu.ie3.datamodel.io.naming.DataSourceMetaInformation; | ||
| import java.util.Objects; | ||
| import java.util.UUID; | ||
|
|
||
| /** Specific meta information, that can be derived from a individual time series file */ | ||
| public class IndividualTimeSeriesMetaInformation extends FileNameMetaInformation { | ||
| /** Specific meta information, that can be derived from an individual time series file */ | ||
| public class IndividualTimeSeriesMetaInformation extends DataSourceMetaInformation { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know, it's not your fault (I introduced this flaw), but I'm not super happy with the fact, that this is not an abstract class. It would be better, if you could only instantiate One opportunity, I would like you to check out, is to check, if What do you think? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To the second part: A counter-argument would be here, that multiple There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To the first point: |
||
| private final ColumnScheme columnScheme; | ||
|
|
||
| public IndividualTimeSeriesMetaInformation(UUID uuid, ColumnScheme columnScheme) { | ||
|
|
@@ -25,9 +25,8 @@ public ColumnScheme getColumnScheme() { | |
| @Override | ||
| public boolean equals(Object o) { | ||
| if (this == o) return true; | ||
| if (!(o instanceof IndividualTimeSeriesMetaInformation)) return false; | ||
| if (!(o instanceof IndividualTimeSeriesMetaInformation that)) return false; | ||
| if (!super.equals(o)) return false; | ||
| IndividualTimeSeriesMetaInformation that = (IndividualTimeSeriesMetaInformation) o; | ||
| return columnScheme == that.columnScheme; | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,14 +3,14 @@ | |
| * Institute of Energy Systems, Energy Efficiency and Energy Economics, | ||
| * Research group Distribution grid planning and operation | ||
| */ | ||
| package edu.ie3.datamodel.io.csv.timeseries; | ||
| package edu.ie3.datamodel.io.naming.timeseries; | ||
|
|
||
| import edu.ie3.datamodel.io.csv.FileNameMetaInformation; | ||
| import edu.ie3.datamodel.io.naming.DataSourceMetaInformation; | ||
| import java.util.Objects; | ||
| import java.util.UUID; | ||
|
|
||
| /** Specific meta information, that can be derived from a load profile time series file */ | ||
| public class LoadProfileTimeSeriesMetaInformation extends FileNameMetaInformation { | ||
| public class LoadProfileTimeSeriesMetaInformation extends DataSourceMetaInformation { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same applies as for |
||
| private final String profile; | ||
|
|
||
| public LoadProfileTimeSeriesMetaInformation(UUID uuid, String profile) { | ||
|
|
@@ -25,9 +25,8 @@ public String getProfile() { | |
| @Override | ||
| public boolean equals(Object o) { | ||
| if (this == o) return true; | ||
| if (!(o instanceof LoadProfileTimeSeriesMetaInformation)) return false; | ||
| if (!(o instanceof LoadProfileTimeSeriesMetaInformation that)) return false; | ||
| if (!super.equals(o)) return false; | ||
| LoadProfileTimeSeriesMetaInformation that = (LoadProfileTimeSeriesMetaInformation) o; | ||
| return profile.equals(that.profile); | ||
| } | ||
|
|
||
|
|
||
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.
The class nearly has the same outline as
CsvIndividualTimeSeriesMetaInformation. Maybe we could also merge both classes? Instead oftableNameandfullFilePathsomething likelocation?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.
I like your suggestion, however I also like having a distinction between csv and sql meta information