Skip to content

Conversation

johanneshiry
Copy link
Member

resolves #78

@johanneshiry johanneshiry added the enhancement New feature or request label Mar 31, 2020
@johanneshiry johanneshiry added this to the Version 1.0 milestone Mar 31, 2020
@johanneshiry johanneshiry requested a review from ckittl March 31, 2020 12:15
@johanneshiry johanneshiry self-assigned this Mar 31, 2020
@codecov
Copy link

codecov bot commented Mar 31, 2020

Codecov Report

Merging #92 into master will increase coverage by 0.29%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #92      +/-   ##
============================================
+ Coverage     61.52%   61.82%   +0.29%     
- Complexity      777      793      +16     
============================================
  Files           182      184       +2     
  Lines          3413     3455      +42     
  Branches        430      436       +6     
============================================
+ Hits           2100     2136      +36     
  Misses         1188     1188              
- Partials        125      131       +6
Impacted Files Coverage Δ Complexity Δ
...in/java/edu/ie3/datamodel/io/sink/CsvFileSink.java 87.09% <0%> (-6.09%) 19% <0%> (+4%)
...3/datamodel/models/input/MeasurementUnitInput.java 40% <0%> (ø) 6% <0%> (ø) ⬇️
...el/models/input/system/SystemParticipantInput.java 33.33% <0%> (ø) 3% <0%> (ø) ⬇️
...del/models/input/connector/Transformer3WInput.java 46.87% <0%> (ø) 8% <0%> (ø) ⬇️
...ava/edu/ie3/datamodel/models/input/AssetInput.java 57.89% <0%> (ø) 7% <0%> (+1%) ⬆️
...del/models/input/connector/Transformer2WInput.java 35.71% <0%> (ø) 4% <0%> (ø) ⬇️
...edu/ie3/datamodel/models/input/system/EvInput.java 28.57% <0%> (ø) 2% <0%> (ø) ⬇️
...e3/datamodel/models/input/system/StorageInput.java 35.29% <0%> (ø) 3% <0%> (ø) ⬇️
...e3/datamodel/models/input/connector/LineInput.java 38.46% <0%> (ø) 5% <0%> (ø) ⬇️
...du/ie3/datamodel/models/input/system/ChpInput.java 40% <0%> (ø) 5% <0%> (ø) ⬇️
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 40f93a1...f6d8106. Read the comment docs.

@johanneshiry johanneshiry changed the title fixed documentation Extractor for nested models Mar 31, 2020
@johanneshiry johanneshiry marked this pull request as ready for review April 1, 2020 07:41
Copy link
Member

@ckittl ckittl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Despite my comments, your and my approach in #89 differ, especially in terms of the role of the DataSink. Let's discuss to get the best solution.

* @version 0.1
* @since 31.03.20
*/
public class Extractor {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest to have a simple static class. So then you could save the instantiation.

Suggested change
public class Extractor {
public class NestedEntityUtils {
public NestedEntityUtils() {
throw new IllegalStateException("Do not instantiate static utility class");
}

this.extractedEntities = extractElements(nestedEntity);
}

private List<InputEntity> extractElements(Nested nestedEntity) throws ExtractorException {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private List<InputEntity> extractElements(Nested nestedEntity) throws ExtractorException {
private Set<InputEntity> extractElements(Collection<Nested> nestedEntities) throws ExtractorException {
return Collections.unmodifiableSet(nestedEntities.stream().map(nested -> {
if (nestedEntities instanceof Node) {
return Collections.singletonList(((Node) nestedEntities).getNode());
} else if (nestedEntities instanceof NodeC) {
return Collections.singletonList(((NodeC) nestedEntities).getNodeC());
} else if (nestedEntities instanceof Nodes) {
return Arrays.asList(((Nodes) nestedEntities).getNodeA(), ((Nodes) nestedEntities).getNodeB());
} else if (nestedEntities instanceof Type) {
return Collections.singletonList(((Type) nestedEntities).getType());
} else {
return new ArrayList<InputEntity>();
}
}).flatMap(List::stream).collect(Collectors.toSet()));
}

This automatically would ensure, that duplicated nodes etc. are only apparent once.

* @version 0.1
* @since 31.03.20
*/
public interface Nested {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public interface Nested {}
public interface NestedEntity {}

* @version 0.1
* @since 31.03.20
*/
public interface Node extends Nested {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about pimpin the Nodes interface and letting go Node and NodeC? See comments below.

* @version 0.1
* @since 31.03.20
*/
public interface Nodes extends Nested {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public interface Nodes extends Nested {
public interface WithNestedNodes extends Nested {

It is not really an Interface describing nodes conceptually, but an entity, that has nodes.

Comment on lines +21 to +23
NodeInput getNodeA();

NodeInput getNodeB();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
NodeInput getNodeA();
NodeInput getNodeB();
Collection<NodeInput> getNodes();

@ckittl ckittl merged commit ae69cc9 into master Apr 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extractor for nested Models into their own collections for persistence
2 participants