Json to data model #29
Conversation
in progress. NOTE that the parser will introduce a new Maven dependency towards "org.json"
Improved documentation.
Renamed JsonParser to JsonConverter since it represents better what the class actually does. Renamed variables to better represent, what they contain. === Assertions === Removed unneccesary assertions. Added missing assertions. === Code general === Added further Methods so at least the top-level converting is complete. Finer granularity is still work in progress. === Documentation === Added and improved documentation.
handling of other methods. Also added documentation.
in progress. NOTE that the parser will introduce a new Maven dependency towards "org.json"
Improved documentation.
Renamed JsonParser to JsonConverter since it represents better what the class actually does. Renamed variables to better represent, what they contain. === Assertions === Removed unneccesary assertions. Added missing assertions. === Code general === Added further Methods so at least the top-level converting is complete. Finer granularity is still work in progress. === Documentation === Added and improved documentation.
handling of other methods. Also added documentation.
…a-Toolkit.git into jsonToDataModel
in progress. NOTE that the parser will introduce a new Maven dependency towards "org.json"
Improved documentation.
Renamed JsonParser to JsonConverter since it represents better what the class actually does. Renamed variables to better represent, what they contain. === Assertions === Removed unneccesary assertions. Added missing assertions. === Code general === Added further Methods so at least the top-level converting is complete. Finer granularity is still work in progress. === Documentation === Added and improved documentation.
handling of other methods. Also added documentation.
in progress. NOTE that the parser will introduce a new Maven dependency towards "org.json"
Improved documentation.
Renamed JsonParser to JsonConverter since it represents better what the class actually does. Renamed variables to better represent, what they contain. === Assertions === Removed unneccesary assertions. Added missing assertions. === Code general === Added further Methods so at least the top-level converting is complete. Finer granularity is still work in progress. === Documentation === Added and improved documentation. Conflicts: wdtk-dumpfiles/src/main/java/org/wikidata/wdtk/dumpfiles/JsonConverter.java
Renamed JsonParser to JsonConverter since it represents better what the class actually does. Renamed variables to better represent, what they contain. === Assertions === Removed unneccesary assertions. Added missing assertions. === Code general === Added further Methods so at least the top-level converting is complete. Finer granularity is still work in progress. === Documentation === Added and improved documentation. Conflicts: wdtk-dumpfiles/src/main/java/org/wikidata/wdtk/dumpfiles/JsonConverter.java
…a-Toolkit into jsonToDataModel Conflicts: wdtk-dumpfiles/src/main/java/org/wikidata/wdtk/dumpfiles/JsonConverter.java
purposes. Samples are included.
for debugging purposes.
Conflicts: wdtk-dumpfiles/pom.xml Hopefully this does it now…
|
Should be fine to merge now. I dont see any remaining problems. |
* Renamed methods according to style guide * Do not catch arbitrary Exceptions but only the ones we know of * Move inline comments into JavaDoc
* Removed unused default factory case and according documentation * Removed documentation for entity prefixes; those are documented in the datamodel, where they are defined
I forgot two methods in the earlier commit.
* Fixed name to make sense * Moved inline comments to JavaDoc
* Fixed spelling of method * Changed JavaDoc to be consistent with other get...Value methods * Added FIXME for an open problem as discussed previously in email
* Unified JavaDoc documentations * Document method behavior and default assumptions in JavaDoc rather than inline * Changed defaults for coordinates to use a default globe (rather than "") and to default precision to maximal precision rather than to degrees * Removed several unclear inline comments
* Fixed names of some parameters ("jsonMainSnak")
* Unified JavaDocs
* Moved inline comments to JavaDoc
* Simplified code where a local variable is returned one line after it
is declared
* Simplified some inline comments
Do not tolerate broken qualifiers here. Throw an exception.
* Renamed to getStatementGroups as it should be * Created a new method getStatement to separate that code * Renamed parameters for consistency with other methods * Updated documentation to be consistent with other methods
As discussed earlier ...
* Applied uniform naming conventions * Unified documentation * Moved inline comments to JavaDoc
* Removed unnecessary support for extracting entity from JSON * Removed all methods that became obsolete due to this * Updated test cases to provide default ids * Cleaned up some test cases that did multiple tests in one method * Unified JavaDocs
* Removed a lot of unnecessary code * Use Java resource loading instead of File access (which would have failed in a JAR) * Marked test cases that don't test anything with FIXMEs
* Make members non-static and recreate them for each test (to ensure that there is never a dependency between tests, even if some objects have internal state) * Use string constants where appropriate instead of non-final static or final non-static strings
* Corrected test case to check for proper datatype IRI * Corrected code to pass test case * Added anothe FIXME in JsonConverter and removed a TODO
|
@fer-rum I have still done quite a few fixes in the code after all. Please have a look at my commits to see what was amiss. Quite a few of the issues were things we had discussed before (LinkedList, violations of naming conventions, inline comments that should have been in JavaDoc). You need to be more careful to fix a problem in all places where it occurs, not just in one or two instances where I explicitly comment on. I fixed the property datatype handling as we had discussed on the mailing list, and I corrected the test case (which was testing for the wrong property type as well, hence was passing in spite of the error). I also removed the null-entity handling as discussed. This simplified the code a lot: it turned out that four methods where only there to support this unnecessary case. I cleaned up the test code. A lot of code there was not needed and could be written much shorter without needing any extra classes. I use resource loading instead of File access to get the json now (I think we had discussed this too; File access does not work in general to get resources stored under src). A number of test methods where doing several completely unrelated tests (e.g., loading several files and checking each for exceptions); this must be avoided. After simplifying and splitting the tests, it is obvious now that many of the tests do not test anything (other than that there are no exceptions happening). We should change the tests to really test something specific that was not tested already. Some of the current test coverage may be an illusion that is created by running functions on a lot of data although we do not check that the results are correct at all. |
|
Btw, this merge request addressed issue #10. |
|
@mkroetzsch Sorry for the extra work. I will try to improve. |
The code and its documentation are complete, so I open this up for review and discussion.