ralfstx / minimal-json

A fast and small JSON parser and writer for Java
MIT License
732 stars 186 forks source link

Optionals #95

Open CLOVIS-AI opened 6 years ago

CLOVIS-AI commented 6 years ago

Adding getters using Optionals to be more "up-to-date" with Java 8 (see issue #94 for the discussion)

CLOVIS-AI commented 6 years ago

There is currently a problem with org.codehaus.mojo.signature:java15:1.0 that dislikes every reference to any Optional object; stacktrace:

JsonObject.java:595: Undefined reference: java.util.Optional java.util.Optional.ofNullable(Object)

A line alike this one appears for each use of Optional, OptionalInt...

sroughley commented 6 years ago

I guess this is because optionals were new in Java 1.8?

CLOVIS-AI commented 6 years ago

That's probably why, but Travis is not having it, probably because of some dependency. I'm not knowledgeable in any way in the Travis ecosystem, so I don't think I will be able to solve that one, sadly... I think someone better at it might need to intervene

Otherwise, I think the work is fully done, all the getters in JsonObject were done and I didn't see any other that would need Optionals at this point (it wouldn't make sense for JsonArray#get).

nbartels commented 5 years ago

The problem is in the pom.xml.

There is a build plugin called animal-sniffer-maven-plugin. In this plugin java 1.5 is checked. I think (but without trying it) you have to make this change:

<plugin>
   <groupId>org.codehaus.mojo</groupId>
   <artifactId>animal-sniffer-maven-plugin</artifactId>
   <version>1.13</version>
   <configuration>
     <signature>
      <groupId>org.codehaus.mojo.signature</groupId>
       <artifactId>java18</artifactId>
       <version>1.0</version>
     </signature>
   </configuration>
   <executions>
     <execution>
       <id>ensure-java-1.8-class-library</id>
       <phase>test</phase>
       <goals>
         <goal>check</goal>
       </goals>
     </execution>
   </executions>
</plugin>

Additionally the maven-bundle-plugin plugin needs to be changed. The line <Bundle-RequiredExecutionEnvironment>J2SE-1.5</Bundle-RequiredExecutionEnvironment>should be changed to 1.8. The value seems to be JavaSE-1.8.

With these changes the optionals should work. If you like you can try this. Otherwise I can try to put them on top of your changes as soon as I get some time ;)

CLOVIS-AI commented 5 years ago

I'm really busy right now, at most I'll be able to do it in ~week.

Also, it would be nice to add @NotNull and @Nullable annotations to improve Kotlin compatibility: a short list of compatible extensions is found here and the full list can be found here (in the source code).

nbartels commented 5 years ago

Additionally some null checks, in the Json class for example, can be replaced by Objects.requireNonNull.

And I think as soon as the source level is switched to 8 more cool java 8 features can be used, that make the code even better.