You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@graphar.apache.org by "Jarvx200 (via GitHub)" <gi...@apache.org> on 2025/09/03 17:03:55 UTC

[PR] refactor: `EdgeInfo` creation using access-level compliant builder, a… [incubator-graphar]

Jarvx200 opened a new pull request, #743:
URL: https://github.com/apache/incubator-graphar/pull/743

   <!--
   Thanks for contributing to GraphAr.
   If this is your first pull request you can find detailed information on [CONTRIBUTING.md](https://github.com/apache/graphar/blob/main/CONTRIBUTING.md)
   
   The Apache GraphAr (incubating) community has restrictions on the naming of pr title. You can find instructions in
   [CONTRIBUTING.md](https://github.com/apache/graphar/blob/main/CONTRIBUTING.md#title) too.
   -->
   
   ### Reason for this PR
   
   Boiler plate heavy telescoping constructor inside `EdgeInfo`
   <!-- 
   Why are you proposing this change? If this is already tracked in an issue, please link to the issue here.
   Explaining clearly why this change is beneficial is important for the reviewers to understand the context.
   -->
   
   
   ### What changes are included in this PR?
   
   Refactor of telescoping constructor to builder pattern while keeping API access levels and compatibility.
   
   ### Are these changes tested?
   
   Yes, using the new builder in `TestUtil` and running the dependent `GraphSaverTest`
   
   ### Are there any user-facing changes?
   
   No
   
   
   


-- 
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: commits-unsubscribe@graphar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@graphar.apache.org
For additional commands, e-mail: commits-help@graphar.apache.org


Re: [PR] refactor: `EdgeInfo` creation using access-level compliant builder, a… [incubator-graphar]

Posted by "Jarvx200 (via GitHub)" <gi...@apache.org>.
Jarvx200 commented on PR #743:
URL: https://github.com/apache/incubator-graphar/pull/743#issuecomment-3250072914

   If the pattern is correct, I can continue by deprecating the old constructors and switching to the new method


-- 
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: commits-unsubscribe@graphar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@graphar.apache.org
For additional commands, e-mail: commits-help@graphar.apache.org


Re: [PR] refactor: `EdgeInfo` creation using access-level compliant builder, a… [incubator-graphar]

Posted by "Jarvx200 (via GitHub)" <gi...@apache.org>.
Jarvx200 commented on PR #743:
URL: https://github.com/apache/incubator-graphar/pull/743#issuecomment-3255034536

   Changes:
   - Added static factory to builder as requested 
   - Added methods for list types to get appended (Note: since `PropertyGroups` is immutable reducing was needed to get the last instance with all the appended groups)
   - Added support to construct `EdgeTriplet` with variables inside builder


-- 
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: commits-unsubscribe@graphar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@graphar.apache.org
For additional commands, e-mail: commits-help@graphar.apache.org


Re: [PR] refactor: `EdgeInfo` creation using access-level compliant builder, a… [incubator-graphar]

Posted by "Jarvx200 (via GitHub)" <gi...@apache.org>.
Jarvx200 commented on PR #743:
URL: https://github.com/apache/incubator-graphar/pull/743#issuecomment-3259547597

   Hello, I went through all the proposed issues and provided fixes and testing!


-- 
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: commits-unsubscribe@graphar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@graphar.apache.org
For additional commands, e-mail: commits-help@graphar.apache.org


Re: [PR] refactor: `EdgeInfo` creation using access-level compliant builder, a… [incubator-graphar]

Posted by "Jarvx200 (via GitHub)" <gi...@apache.org>.
Jarvx200 commented on PR #743:
URL: https://github.com/apache/incubator-graphar/pull/743#issuecomment-3253623147

   Also, [Lombok](https://projectlombok.org/) could be useful but it has some limitations and edge-cases, one example being what you described in :
   > Can we add method sourceType(String srcType), edgeType(String edgeType), destinationType(String dstType)? I am not sure, @Thespica
   


-- 
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: commits-unsubscribe@graphar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@graphar.apache.org
For additional commands, e-mail: commits-help@graphar.apache.org


Re: [PR] refactor: `EdgeInfo` creation using access-level compliant builder, a… [incubator-graphar]

Posted by "yangxk1 (via GitHub)" <gi...@apache.org>.
yangxk1 commented on code in PR #743:
URL: https://github.com/apache/incubator-graphar/pull/743#discussion_r2328770525


##########
maven-projects/info/src/main/java/org/apache/graphar/info/EdgeInfo.java:
##########
@@ -51,6 +53,192 @@ public class EdgeInfo {
     private final PropertyGroups propertyGroups;
     private final VersionInfo version;
 
+    public static EdgeInfoBuilder builder(){
+        return new EdgeInfoBuilder();
+    }
+
+    public static final class EdgeInfoBuilder {
+        private EdgeTriplet edgeTriplet;
+        private long chunkSize;
+        private long srcChunkSize;
+        private long dstChunkSize;
+        private boolean directed;
+        private String prefix;
+        private Map<AdjListType, AdjacentList> adjacentLists;
+        private PropertyGroups propertyGroups;
+        private VersionInfo version;
+
+        private List<AdjacentList> adjacentListsAsListTemp;
+        private List<PropertyGroup> propertyGroupsAsListTemp;
+
+        private String srcType;
+        private String edgeType;
+        private String dstType;
+
+
+        private EdgeInfoBuilder() {}
+
+
+        public EdgeInfoBuilder srcType(String srcType) {
+            this.srcType = srcType;
+            return this;
+        }
+
+        public EdgeInfoBuilder edgeType(String edgeType) {
+            this.edgeType = edgeType;
+            return this;
+        }
+        public EdgeInfoBuilder dstType(String dstType) {
+            this.dstType = dstType;
+            return this;
+        }
+
+        public EdgeInfoBuilder edgeTriplet(String srcType, String edgeType, String dstType) {
+            this.edgeTriplet = new EdgeTriplet(srcType, edgeType, dstType);
+            return this;
+        }
+
+
+        private EdgeInfoBuilder edgeTriplet(EdgeTriplet edgeTriplet) {
+            this.edgeTriplet = edgeTriplet;
+            return this;
+        }
+
+        public EdgeInfoBuilder chunkSize(long chunkSize) {
+            this.chunkSize = chunkSize;
+            return this;
+        }
+
+        public EdgeInfoBuilder srcChunkSize(long srcChunkSize) {
+            this.srcChunkSize = srcChunkSize;
+            return this;
+        }
+
+        public EdgeInfoBuilder dstChunkSize(long dstChunkSize) {
+            this.dstChunkSize = dstChunkSize;
+            return this;
+        }
+
+        public EdgeInfoBuilder directed(boolean directed) {
+            this.directed = directed;
+            return this;
+        }
+
+        public EdgeInfoBuilder prefix(String prefix) {
+            this.prefix = prefix;
+            return this;
+        }
+
+        public EdgeInfoBuilder addAdjacentList(AdjacentList adjacentList) {
+            if(adjacentListsAsListTemp == null){
+                adjacentListsAsListTemp = new ArrayList<>();
+                }
+            adjacentListsAsListTemp.add(adjacentList);
+            return this;
+        }
+
+        public EdgeInfoBuilder adjacentLists(List<AdjacentList> adjacentListsAsList) {
+            if(adjacentListsAsListTemp == null) {
+                adjacentListsAsListTemp = new ArrayList<>();
+            }
+            this.adjacentListsAsListTemp.addAll(adjacentListsAsList);
+            return this;
+        }
+
+        private EdgeInfoBuilder adjacentLists(Map<AdjListType, AdjacentList> adjacentLists) {

Review Comment:
   Also public



##########
maven-projects/info/src/main/java/org/apache/graphar/info/EdgeInfo.java:
##########
@@ -51,6 +53,192 @@ public class EdgeInfo {
     private final PropertyGroups propertyGroups;
     private final VersionInfo version;
 
+    public static EdgeInfoBuilder builder(){
+        return new EdgeInfoBuilder();
+    }
+
+    public static final class EdgeInfoBuilder {
+        private EdgeTriplet edgeTriplet;
+        private long chunkSize;
+        private long srcChunkSize;
+        private long dstChunkSize;
+        private boolean directed;
+        private String prefix;
+        private Map<AdjListType, AdjacentList> adjacentLists;
+        private PropertyGroups propertyGroups;
+        private VersionInfo version;
+
+        private List<AdjacentList> adjacentListsAsListTemp;
+        private List<PropertyGroup> propertyGroupsAsListTemp;
+
+        private String srcType;
+        private String edgeType;
+        private String dstType;
+
+
+        private EdgeInfoBuilder() {}
+
+
+        public EdgeInfoBuilder srcType(String srcType) {
+            this.srcType = srcType;
+            return this;
+        }
+
+        public EdgeInfoBuilder edgeType(String edgeType) {
+            this.edgeType = edgeType;
+            return this;
+        }
+        public EdgeInfoBuilder dstType(String dstType) {
+            this.dstType = dstType;
+            return this;
+        }
+
+        public EdgeInfoBuilder edgeTriplet(String srcType, String edgeType, String dstType) {
+            this.edgeTriplet = new EdgeTriplet(srcType, edgeType, dstType);
+            return this;
+        }
+
+
+        private EdgeInfoBuilder edgeTriplet(EdgeTriplet edgeTriplet) {
+            this.edgeTriplet = edgeTriplet;
+            return this;
+        }
+
+        public EdgeInfoBuilder chunkSize(long chunkSize) {
+            this.chunkSize = chunkSize;
+            return this;
+        }
+
+        public EdgeInfoBuilder srcChunkSize(long srcChunkSize) {
+            this.srcChunkSize = srcChunkSize;
+            return this;
+        }
+
+        public EdgeInfoBuilder dstChunkSize(long dstChunkSize) {
+            this.dstChunkSize = dstChunkSize;
+            return this;
+        }
+
+        public EdgeInfoBuilder directed(boolean directed) {
+            this.directed = directed;
+            return this;
+        }
+
+        public EdgeInfoBuilder prefix(String prefix) {
+            this.prefix = prefix;
+            return this;
+        }
+
+        public EdgeInfoBuilder addAdjacentList(AdjacentList adjacentList) {
+            if(adjacentListsAsListTemp == null){
+                adjacentListsAsListTemp = new ArrayList<>();
+                }
+            adjacentListsAsListTemp.add(adjacentList);
+            return this;
+        }
+
+        public EdgeInfoBuilder adjacentLists(List<AdjacentList> adjacentListsAsList) {
+            if(adjacentListsAsListTemp == null) {
+                adjacentListsAsListTemp = new ArrayList<>();
+            }
+            this.adjacentListsAsListTemp.addAll(adjacentListsAsList);
+            return this;
+        }
+
+        private EdgeInfoBuilder adjacentLists(Map<AdjListType, AdjacentList> adjacentLists) {
+            this.adjacentLists = adjacentLists;
+            return this;
+        }
+
+        public EdgeInfoBuilder addPropertyGroups(PropertyGroup propertyGroup) {
+            if(propertyGroupsAsListTemp == null)
+                propertyGroupsAsListTemp = new ArrayList<>();
+            propertyGroupsAsListTemp.add(propertyGroup);
+            return this;
+        }
+
+        public EdgeInfoBuilder addPropertyGroups(List<PropertyGroup> propertyGroups) {
+            if(propertyGroupsAsListTemp == null)
+                propertyGroupsAsListTemp = new ArrayList<>();
+            propertyGroupsAsListTemp.addAll(propertyGroups);
+            return this;
+        }
+
+        private EdgeInfoBuilder addPropertyGroups(PropertyGroups propertyGroups) {

Review Comment:
   change the access modifiers and name to `public propertyGroups`



-- 
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: commits-unsubscribe@graphar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@graphar.apache.org
For additional commands, e-mail: commits-help@graphar.apache.org


Re: [PR] refactor: `EdgeInfo` creation using access-level compliant builder, a… [incubator-graphar]

Posted by "yangxk1 (via GitHub)" <gi...@apache.org>.
yangxk1 commented on code in PR #743:
URL: https://github.com/apache/incubator-graphar/pull/743#discussion_r2320651660


##########
maven-projects/info/src/main/java/org/apache/graphar/info/EdgeInfo.java:
##########
@@ -51,6 +51,103 @@ public class EdgeInfo {
     private final PropertyGroups propertyGroups;
     private final VersionInfo version;
 
+    public static final class EdgeInfoBuilder {
+        private EdgeTriplet edgeTriplet;
+        private long chunkSize;
+        private long srcChunkSize;
+        private long dstChunkSize;
+        private boolean directed;
+        private String prefix;
+        private Map<AdjListType, AdjacentList> adjacentLists;
+        private PropertyGroups propertyGroups;
+        private VersionInfo version;
+
+        public EdgeInfoBuilder edgeTriplet(String srcType, String edgeType, String dstType) {
+            this.edgeTriplet = new EdgeTriplet(srcType, edgeType, dstType);
+            return this;
+        }
+
+        private EdgeInfoBuilder edgeTriplet(EdgeTriplet edgeTriplet) {
+            this.edgeTriplet = edgeTriplet;
+            return this;
+        }
+
+        public EdgeInfoBuilder chunkSize(long chunkSize) {
+            this.chunkSize = chunkSize;
+            return this;
+        }
+
+        public EdgeInfoBuilder srcChunkSize(long srcChunkSize) {
+            this.srcChunkSize = srcChunkSize;
+            return this;
+        }
+
+        public EdgeInfoBuilder dstChunkSize(long dstChunkSize) {
+            this.dstChunkSize = dstChunkSize;
+            return this;
+        }
+
+        public EdgeInfoBuilder directed(boolean directed) {
+            this.directed = directed;
+            return this;
+        }
+
+        public EdgeInfoBuilder prefix(String prefix) {
+            this.prefix = prefix;
+            return this;
+        }
+
+        public EdgeInfoBuilder adjacentLists(List<AdjacentList> adjacentListsAsList) {
+            this.adjacentLists =
+                    adjacentListsAsList.stream()
+                            .collect(
+                                    Collectors.toUnmodifiableMap(
+                                            AdjacentList::getType, Function.identity()));
+            return this;
+        }
+
+        private EdgeInfoBuilder adjecentLists(Map<AdjListType, AdjacentList> adjacentLists) {

Review Comment:
   adjecentLists -> adjacentLists?
   Can you add some `addXxx` method to List type fileds? e.g., `addAdjacentList()`,`addPropertyGroup()`



##########
maven-projects/info/src/main/java/org/apache/graphar/info/EdgeInfo.java:
##########
@@ -51,6 +51,103 @@ public class EdgeInfo {
     private final PropertyGroups propertyGroups;
     private final VersionInfo version;
 
+    public static final class EdgeInfoBuilder {
+        private EdgeTriplet edgeTriplet;
+        private long chunkSize;
+        private long srcChunkSize;
+        private long dstChunkSize;
+        private boolean directed;
+        private String prefix;
+        private Map<AdjListType, AdjacentList> adjacentLists;
+        private PropertyGroups propertyGroups;
+        private VersionInfo version;
+
+        public EdgeInfoBuilder edgeTriplet(String srcType, String edgeType, String dstType) {

Review Comment:
   Can we add method sourceType(String srcType), edgeType(String edgeType), destinationType(String dstType)?
   I am not sure, @Thespica 



##########
maven-projects/info/src/test/java/org/apache/graphar/info/TestUtil.java:
##########
@@ -111,19 +111,19 @@ public static GraphInfo getLdbcSampleDataSet() {
                 new AdjacentList(AdjListType.ordered_by_dest, FileType.CSV, "ordered_by_dest/");
         Property creationDate = new Property("creationDate", DataType.STRING, false, false);
         PropertyGroup pg3 = new PropertyGroup(List.of(creationDate), FileType.CSV, "creationDate/");
+
         EdgeInfo knows =
-                new EdgeInfo(
-                        "person",
-                        "knows",
-                        "person",
-                        1024,
-                        100,
-                        100,
-                        false,
-                        "edge/person_knows_person/",
-                        "gar/v1",
-                        List.of(orderedBySource, orderedByDest),
-                        List.of(pg3));
+                new EdgeInfo.EdgeInfoBuilder()

Review Comment:
   Add a parameterless constructor to `EdgeInfoBuilder` and add a static method
    `static EdgeInfoBuilder builder()` to EdgeInfo. 
   Than it can be changed to
   ```
   EdgeInfo knows = EdgeInfo.builder().edgeTriplet()...build()
   ````
   
   



-- 
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: commits-unsubscribe@graphar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@graphar.apache.org
For additional commands, e-mail: commits-help@graphar.apache.org


Re: [PR] refactor: `EdgeInfo` creation using access-level compliant builder, a… [incubator-graphar]

Posted by "yangxk1 (via GitHub)" <gi...@apache.org>.
yangxk1 commented on code in PR #743:
URL: https://github.com/apache/incubator-graphar/pull/743#discussion_r2323948116


##########
maven-projects/info/src/main/java/org/apache/graphar/info/EdgeInfo.java:
##########
@@ -20,9 +20,7 @@
 package org.apache.graphar.info;
 
 import java.io.IOException;
-import java.util.List;
-import java.util.Map;
-import java.util.Optional;
+import java.util.*;

Review Comment:
   Import specific classes, rather than using *.



##########
maven-projects/info/src/main/java/org/apache/graphar/info/EdgeInfo.java:
##########
@@ -51,6 +49,174 @@ public class EdgeInfo {
     private final PropertyGroups propertyGroups;
     private final VersionInfo version;
 
+    public static final class EdgeInfoBuilder {
+        private EdgeTriplet edgeTriplet;
+        private long chunkSize;
+        private long srcChunkSize;
+        private long dstChunkSize;
+        private boolean directed;
+        private String prefix;
+        private Map<AdjListType, AdjacentList> adjacentLists;
+        private PropertyGroups propertyGroups;
+        private VersionInfo version;
+
+        private List<AdjacentList> adjacentListsAsListTemp;
+        private List<PropertyGroup> propertyGroupsAsListTemp;
+
+        private String srcType;
+        private String edgeType;
+        private String dstType;
+
+        public static EdgeInfoBuilder builder() {

Review Comment:
   move the `builder()` method to `EdgeInfo` class



##########
maven-projects/info/src/test/java/org/apache/graphar/info/TestUtil.java:
##########
@@ -111,19 +111,19 @@ public static GraphInfo getLdbcSampleDataSet() {
                 new AdjacentList(AdjListType.ordered_by_dest, FileType.CSV, "ordered_by_dest/");
         Property creationDate = new Property("creationDate", DataType.STRING, false, false);
         PropertyGroup pg3 = new PropertyGroup(List.of(creationDate), FileType.CSV, "creationDate/");
+
         EdgeInfo knows =
-                new EdgeInfo(
-                        "person",
-                        "knows",
-                        "person",
-                        1024,
-                        100,
-                        100,
-                        false,
-                        "edge/person_knows_person/",
-                        "gar/v1",
-                        List.of(orderedBySource, orderedByDest),
-                        List.of(pg3));
+                EdgeInfo.EdgeInfoBuilder.builder()

Review Comment:
   add some cases for verification failed



##########
maven-projects/info/src/main/java/org/apache/graphar/info/EdgeInfo.java:
##########
@@ -51,6 +49,174 @@ public class EdgeInfo {
     private final PropertyGroups propertyGroups;
     private final VersionInfo version;
 
+    public static final class EdgeInfoBuilder {
+        private EdgeTriplet edgeTriplet;
+        private long chunkSize;
+        private long srcChunkSize;
+        private long dstChunkSize;
+        private boolean directed;
+        private String prefix;
+        private Map<AdjListType, AdjacentList> adjacentLists;
+        private PropertyGroups propertyGroups;
+        private VersionInfo version;
+
+        private List<AdjacentList> adjacentListsAsListTemp;
+        private List<PropertyGroup> propertyGroupsAsListTemp;
+
+        private String srcType;
+        private String edgeType;
+        private String dstType;
+
+        public static EdgeInfoBuilder builder() {
+            return new EdgeInfoBuilder();
+        }
+
+        private EdgeInfoBuilder() {}
+
+
+        public EdgeInfoBuilder srcType(String srcType) {
+            this.srcType = srcType;
+            return this;
+        }
+
+        public EdgeInfoBuilder edgeType(String edgeType) {
+            this.edgeType = edgeType;
+            return this;
+        }
+        public EdgeInfoBuilder dstType(String dstType) {
+            this.dstType = dstType;
+            return this;
+        }
+
+        public EdgeInfoBuilder edgeTriplet(String srcType, String edgeType, String dstType) {
+            this.edgeTriplet = new EdgeTriplet(srcType, edgeType, dstType);
+            return this;
+        }
+
+
+        private EdgeInfoBuilder edgeTriplet(EdgeTriplet edgeTriplet) {
+            this.edgeTriplet = edgeTriplet;
+            return this;
+        }
+
+        public EdgeInfoBuilder chunkSize(long chunkSize) {
+            this.chunkSize = chunkSize;
+            return this;
+        }
+
+        public EdgeInfoBuilder srcChunkSize(long srcChunkSize) {
+            this.srcChunkSize = srcChunkSize;
+            return this;
+        }
+
+        public EdgeInfoBuilder dstChunkSize(long dstChunkSize) {
+            this.dstChunkSize = dstChunkSize;
+            return this;
+        }
+
+        public EdgeInfoBuilder directed(boolean directed) {
+            this.directed = directed;
+            return this;
+        }
+
+        public EdgeInfoBuilder prefix(String prefix) {
+            this.prefix = prefix;
+            return this;
+        }
+
+        public EdgeInfoBuilder addAdjacentList(AdjacentList adjacentList) {
+            if(adjacentListsAsListTemp == null)
+                adjacentListsAsListTemp = new ArrayList<>();
+            adjacentListsAsListTemp.add(adjacentList);
+            return this;
+        }
+
+        public EdgeInfoBuilder adjacentLists(List<AdjacentList> adjacentListsAsList) {
+            if(adjacentListsAsListTemp == null)
+                adjacentListsAsListTemp = new ArrayList<>();
+            this.adjacentListsAsListTemp.addAll(adjacentListsAsList);
+            return this;
+        }
+
+        private EdgeInfoBuilder adjacentLists(Map<AdjListType, AdjacentList> adjacentLists) {
+            this.adjacentLists = adjacentLists;
+            return this;
+        }
+
+        public EdgeInfoBuilder addPropertyGroups(PropertyGroup propertyGroup) {
+            if(propertyGroupsAsListTemp == null)
+                propertyGroupsAsListTemp = new ArrayList<>();
+            propertyGroupsAsListTemp.add(propertyGroup);
+            return this;
+        }
+
+        public EdgeInfoBuilder propertyGroups(List<PropertyGroup> propertyGroups) {

Review Comment:
   rename to `addPropertyGroups` may be better



##########
maven-projects/info/src/main/java/org/apache/graphar/info/EdgeInfo.java:
##########
@@ -51,6 +49,174 @@ public class EdgeInfo {
     private final PropertyGroups propertyGroups;
     private final VersionInfo version;
 
+    public static final class EdgeInfoBuilder {
+        private EdgeTriplet edgeTriplet;
+        private long chunkSize;
+        private long srcChunkSize;
+        private long dstChunkSize;
+        private boolean directed;
+        private String prefix;
+        private Map<AdjListType, AdjacentList> adjacentLists;
+        private PropertyGroups propertyGroups;
+        private VersionInfo version;
+
+        private List<AdjacentList> adjacentListsAsListTemp;
+        private List<PropertyGroup> propertyGroupsAsListTemp;
+
+        private String srcType;
+        private String edgeType;
+        private String dstType;
+
+        public static EdgeInfoBuilder builder() {
+            return new EdgeInfoBuilder();
+        }
+
+        private EdgeInfoBuilder() {}
+
+
+        public EdgeInfoBuilder srcType(String srcType) {
+            this.srcType = srcType;
+            return this;
+        }
+
+        public EdgeInfoBuilder edgeType(String edgeType) {
+            this.edgeType = edgeType;
+            return this;
+        }
+        public EdgeInfoBuilder dstType(String dstType) {
+            this.dstType = dstType;
+            return this;
+        }
+
+        public EdgeInfoBuilder edgeTriplet(String srcType, String edgeType, String dstType) {
+            this.edgeTriplet = new EdgeTriplet(srcType, edgeType, dstType);
+            return this;
+        }
+
+
+        private EdgeInfoBuilder edgeTriplet(EdgeTriplet edgeTriplet) {
+            this.edgeTriplet = edgeTriplet;
+            return this;
+        }
+
+        public EdgeInfoBuilder chunkSize(long chunkSize) {
+            this.chunkSize = chunkSize;
+            return this;
+        }
+
+        public EdgeInfoBuilder srcChunkSize(long srcChunkSize) {
+            this.srcChunkSize = srcChunkSize;
+            return this;
+        }
+
+        public EdgeInfoBuilder dstChunkSize(long dstChunkSize) {
+            this.dstChunkSize = dstChunkSize;
+            return this;
+        }
+
+        public EdgeInfoBuilder directed(boolean directed) {
+            this.directed = directed;
+            return this;
+        }
+
+        public EdgeInfoBuilder prefix(String prefix) {
+            this.prefix = prefix;
+            return this;
+        }
+
+        public EdgeInfoBuilder addAdjacentList(AdjacentList adjacentList) {
+            if(adjacentListsAsListTemp == null)

Review Comment:
   pls do not omit `{}`



##########
maven-projects/info/src/main/java/org/apache/graphar/info/EdgeInfo.java:
##########
@@ -51,6 +49,174 @@ public class EdgeInfo {
     private final PropertyGroups propertyGroups;
     private final VersionInfo version;
 
+    public static final class EdgeInfoBuilder {
+        private EdgeTriplet edgeTriplet;
+        private long chunkSize;
+        private long srcChunkSize;
+        private long dstChunkSize;
+        private boolean directed;
+        private String prefix;
+        private Map<AdjListType, AdjacentList> adjacentLists;
+        private PropertyGroups propertyGroups;
+        private VersionInfo version;
+
+        private List<AdjacentList> adjacentListsAsListTemp;
+        private List<PropertyGroup> propertyGroupsAsListTemp;
+
+        private String srcType;
+        private String edgeType;
+        private String dstType;
+
+        public static EdgeInfoBuilder builder() {
+            return new EdgeInfoBuilder();
+        }
+
+        private EdgeInfoBuilder() {}
+
+
+        public EdgeInfoBuilder srcType(String srcType) {
+            this.srcType = srcType;
+            return this;
+        }
+
+        public EdgeInfoBuilder edgeType(String edgeType) {
+            this.edgeType = edgeType;
+            return this;
+        }
+        public EdgeInfoBuilder dstType(String dstType) {
+            this.dstType = dstType;
+            return this;
+        }
+
+        public EdgeInfoBuilder edgeTriplet(String srcType, String edgeType, String dstType) {
+            this.edgeTriplet = new EdgeTriplet(srcType, edgeType, dstType);
+            return this;
+        }
+
+
+        private EdgeInfoBuilder edgeTriplet(EdgeTriplet edgeTriplet) {
+            this.edgeTriplet = edgeTriplet;
+            return this;
+        }
+
+        public EdgeInfoBuilder chunkSize(long chunkSize) {
+            this.chunkSize = chunkSize;
+            return this;
+        }
+
+        public EdgeInfoBuilder srcChunkSize(long srcChunkSize) {
+            this.srcChunkSize = srcChunkSize;
+            return this;
+        }
+
+        public EdgeInfoBuilder dstChunkSize(long dstChunkSize) {
+            this.dstChunkSize = dstChunkSize;
+            return this;
+        }
+
+        public EdgeInfoBuilder directed(boolean directed) {
+            this.directed = directed;
+            return this;
+        }
+
+        public EdgeInfoBuilder prefix(String prefix) {
+            this.prefix = prefix;
+            return this;
+        }
+
+        public EdgeInfoBuilder addAdjacentList(AdjacentList adjacentList) {
+            if(adjacentListsAsListTemp == null)
+                adjacentListsAsListTemp = new ArrayList<>();
+            adjacentListsAsListTemp.add(adjacentList);
+            return this;
+        }
+
+        public EdgeInfoBuilder adjacentLists(List<AdjacentList> adjacentListsAsList) {
+            if(adjacentListsAsListTemp == null)
+                adjacentListsAsListTemp = new ArrayList<>();
+            this.adjacentListsAsListTemp.addAll(adjacentListsAsList);
+            return this;
+        }
+
+        private EdgeInfoBuilder adjacentLists(Map<AdjListType, AdjacentList> adjacentLists) {
+            this.adjacentLists = adjacentLists;
+            return this;
+        }
+
+        public EdgeInfoBuilder addPropertyGroups(PropertyGroup propertyGroup) {
+            if(propertyGroupsAsListTemp == null)
+                propertyGroupsAsListTemp = new ArrayList<>();
+            propertyGroupsAsListTemp.add(propertyGroup);
+            return this;
+        }
+
+        public EdgeInfoBuilder propertyGroups(List<PropertyGroup> propertyGroups) {
+            if(propertyGroupsAsListTemp == null)
+                propertyGroupsAsListTemp = new ArrayList<>();
+            propertyGroupsAsListTemp.addAll(propertyGroups);
+            return this;
+        }
+
+        private EdgeInfoBuilder propertyGroups(PropertyGroups propertyGroups) {
+            this.propertyGroups = propertyGroups;
+            return this;
+        }
+
+        public EdgeInfoBuilder version(String version) {
+            this.version = VersionParser.getVersion(version);
+            return this;
+        }
+
+        public EdgeInfoBuilder version(VersionInfo version) {
+            this.version = version;
+            return this;
+        }
+
+        public EdgeInfo build() {
+            if(adjacentLists == null)
+                adjacentLists = new HashMap<>();
+
+            if(adjacentListsAsListTemp != null)
+                adjacentLists.putAll(
+                    adjacentListsAsListTemp.stream()
+                            .collect(
+                                    Collectors.toUnmodifiableMap(
+                                            AdjacentList::getType, Function.identity())));
+
+            if(propertyGroups == null)
+                propertyGroups = new PropertyGroups(propertyGroupsAsListTemp);
+            else {
+                if(propertyGroupsAsListTemp != null)
+                    propertyGroups = propertyGroupsAsListTemp.stream()
+                        .map(propertyGroups::addPropertyGroupAsNew)
+                        .filter(Optional::isPresent)
+                        .map(Optional::get)
+                        .reduce((first, second) -> second)
+                        .orElse(new PropertyGroups(new ArrayList<>()));
+                else
+                    propertyGroups = new PropertyGroups(new ArrayList<>());
+            }
+
+            if(edgeTriplet == null && srcType != null && edgeType != null && dstType != null)
+                edgeTriplet = new EdgeTriplet(srcType, edgeType, dstType);

Review Comment:
   we should add some verification, e.g., throw exception when `edgeTriplet == null &&  srcType == null`



-- 
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: commits-unsubscribe@graphar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@graphar.apache.org
For additional commands, e-mail: commits-help@graphar.apache.org


Re: [PR] refactor: `EdgeInfo` creation using access-level compliant builder, a… [incubator-graphar]

Posted by "Jarvx200 (via GitHub)" <gi...@apache.org>.
Jarvx200 commented on PR #743:
URL: https://github.com/apache/incubator-graphar/pull/743#issuecomment-3264237116

   Hello, I fixed the issues above and added a missing `final` to `TestUtil`. If there is any further testing required please tell me.


-- 
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: commits-unsubscribe@graphar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@graphar.apache.org
For additional commands, e-mail: commits-help@graphar.apache.org


Re: [PR] refactor: `EdgeInfo` creation using access-level compliant builder, a… [incubator-graphar]

Posted by "Jarvx200 (via GitHub)" <gi...@apache.org>.
Jarvx200 commented on PR #743:
URL: https://github.com/apache/incubator-graphar/pull/743#issuecomment-3253609545

   Ok sure, I will start working on the first two problems, the third one would imply temporary builder fields that will eventually get transformed with null checking to triplet, but I can wait for further insights on the problem and can implement if ok.


-- 
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: commits-unsubscribe@graphar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@graphar.apache.org
For additional commands, e-mail: commits-help@graphar.apache.org


Re: [PR] refactor: `EdgeInfo` creation using access-level compliant builder, a… [incubator-graphar]

Posted by "Jarvx200 (via GitHub)" <gi...@apache.org>.
Jarvx200 commented on PR #743:
URL: https://github.com/apache/incubator-graphar/pull/743#issuecomment-3263920519

   Hello! I will start working on those problems, write some more tests to make sure everything is compliant.


-- 
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: commits-unsubscribe@graphar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@graphar.apache.org
For additional commands, e-mail: commits-help@graphar.apache.org


Re: [PR] feat(java,info): refactor `EdgeInfo` creation using access-level compliant builder, a… [incubator-graphar]

Posted by "yangxk1 (via GitHub)" <gi...@apache.org>.
yangxk1 merged PR #743:
URL: https://github.com/apache/incubator-graphar/pull/743


-- 
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: commits-unsubscribe@graphar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@graphar.apache.org
For additional commands, e-mail: commits-help@graphar.apache.org