SemyonSinchenko commented on code in PR #956:
URL: https://github.com/apache/incubator-graphar/pull/956#discussion_r3802119685
##########
maven-projects/info/src/main/java/org/apache/graphar/info/PropertyGroup.java:
##########
@@ -208,6 +208,19 @@ Optional<PropertyGroups>
addPropertyGroupAsNew(PropertyGroup propertyGroup) {
newProperties));
}
+ Optional<PropertyGroups> removePropertyGroupAsNew(PropertyGroup
propertyGroup) {
+ if (propertyGroup == null || !hasPropertyGroup(propertyGroup)) {
+ return Optional.empty();
+ }
+ return Optional.of(
+ new PropertyGroups(
+ propertyGroupList.stream()
+ .filter(
+ existingPropertyGroup ->
+ existingPropertyGroup !=
propertyGroup)
Review Comment:
```suggestion
!existingPropertyGroup.equals(propertyGroup))
```
##########
maven-projects/info/src/main/java/org/apache/graphar/info/GraphInfo.java:
##########
@@ -283,6 +283,20 @@ public EdgeInfo getEdgeInfo(String srcType, String
edgeType, String dstType) {
return edgeConcat2EdgeInfo.get(EdgeInfo.concat(srcType, edgeType,
dstType));
}
+ public VertexInfo getVertexInfoByIndex(int index) {
+ if (index < 0 || index >= vertexInfos.size()) {
+ return null;
Review Comment:
I think we should be consistent with `getPropertyGroup` and throw
`IllegalArgumentException` instead of returning `null`.
##########
maven-projects/info/src/main/java/org/apache/graphar/info/VertexInfo.java:
##########
@@ -236,6 +236,21 @@ public Optional<VertexInfo>
addPropertyGroupAsNew(PropertyGroup propertyGroup) {
version));
}
+ public Optional<VertexInfo> removePropertyGroupAsNew(PropertyGroup
propertyGroup) {
+ return propertyGroups
+ .removePropertyGroupAsNew(propertyGroup)
Review Comment:
I think we should implement `equals` (`hashCode`) for `PropertyGroup`,
otherwise it is barely working imo (comparing by reference)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
