Github user arjansh commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/179#discussion_r189885903
--- Diff:
neo4j/src/main/java/org/apache/metamodel/neo4j/Neo4jDataContext.java ---
@@ -158,60 +159,79 @@ protected String getMainSchemaName() throws
MetaModelException {
}
public SimpleTableDef[] detectTableDefs() {
- List<SimpleTableDef> tableDefs = new ArrayList<SimpleTableDef>();
-
- String labelsJsonString = _requestWrapper.executeRestRequest(new
HttpGet(_serviceRoot + "/labels"));
-
- JSONArray labelsJsonArray;
+ final List<SimpleTableDef> tableDefs = new ArrayList<>();
+ final String labelsJsonString =
_requestWrapper.executeRestRequest(new HttpGet(_serviceRoot + "/labels"));
+ final JSONArray labelsJsonArray;
+
try {
labelsJsonArray = new JSONArray(labelsJsonString);
+
for (int i = 0; i < labelsJsonArray.length(); i++) {
- String label = labelsJsonArray.getString(i);
+ fillTableDefsFromLabel(labelsJsonArray.getString(i),
tableDefs);
--- End diff --
The `fillTableDefsFromLabel` method doesn't fill the `tableDefs` list, it
just adds one `SimpleTableDef` in case a `SimpleTableDef` can be created for
the given label. I would rather see a method called `getTableDefForLabel` or
`createTableDefFromLabel`, which returns a `SimpleTableDef` (or null in case
none can be created for the given label) and adds it to the `tableDefs` list if
the value is not null. From my point of view, in that manner that method has
one simple responsibility and the code is easier to understand.
---
