Source quality improvements

[davidmoten]

Firstly congrats on putting together a very interesting project, nice work (and a lot of work looks like!).

I browsed the java source for jsweet to gauge the maturity/quality of the code (to see if I should use the project) and I just thought I'd share some impressions that you may or may not choose to act on.

Now that java.util.Optional is part of Java 8 and this is the compile level for the project then I'd suggest that null is not used anywhere and that where Optional items are passed as arguments that guava style Preconditions.checkNotNull checks are made on those arguments.

In companion with the Optional suggestion there is a lot of mutabiltity floating around and it's not needed. Any non-final field is a flag for closer inspection of usage for me and almost all your fields are non-final. For creating instances of something like JSweetTranspiler I'd suggest using a fluent builder that creates immutable instances.

I'd also avoid inheritance where possible (see composition vs inheritance discussions, e.g. in Josh Bloch's Essential Java) and default to marking classes as final so that you don't have to support inheritance yourselves. You could also move classes that have internal only function to an .internal package so it's clear what is in the public API and what is not.

That's my fifty cents for the moment, thanks for the project!

[renaudpawlak]

Hi David. We value a lot developer feedbacks and thanks a lot for taking the time to look at JSweet so closely. Your feedback is interesting and requires some elaborations on what we are doing and our design philosophy with JSweet.

First of all, let me state that JSweet is still a young project at this point and that the V1 RC1 is planned for around mid January. It is normal that people will still meet some difficulties using JSweet out of the box because at this point, although the transpiler core is quite stable, we need more feedback from the community on the plugins (Maven, Gradle, Eclipse, ...).

Regarding what you call "code quality", I am always a little uncomfortable using the word "quality", because it is by far the most generic and less precise terms of all the "-ities" around there. Are we talking about code readability, code maintainability, code type safety, code safety, code reliability, etc...? When I read your feedback, it feels that your focus is on the API usability and how programmers shall use the JSweet transpiler APIs from Java. This aspect is clearly an important aspect and all your remarks can (and eventually will) be used to improve JSweet. But although it is important, it was clearly not our main concern when designing and implementing JSweet. Let me explain why.

JSweet is a transpiler. I is not a framework. As such, the primary use case of JSweet is using it through the command line launcher or any of the plugins (Maven, Gradle, Eclipse). What the programmers shall see of JSweet are the launching options of JSweet, not the the Java APIs. Using JSweet through its programming APIs from Java, is at this point a secondary use case for most users.

So, JSweet was mainly designed and coded to ensure maximum safety in the JavaScript code generation. Therefore, a lot of work was done on the design and on the architecture, which is very original an consists of cross validating the JSweet programs against the Java AND the TypeScript API, which brings to the programmers 2 safety nets when transpiling JSweet programs (you can read my papers that explain this, but I will eventually blog on this too). Also, a lot of work was done on the test suite, to ensure that most of the JSweet language features get covered by the tests. This goes along with a clear language specification document so that JSweet users know how to write JSweet programs and what to expect when running them.

As a consequence, our main priorities to improve JSweet's quality (or at leat what matters to us, i.e. our primary Use Case) are the following:

• improve the tests accordingly to the language specification
• add a code coverage tool in the build so that we get instant feed back on what transpiler feature is well tested or not
• set up an online service to allow the users to easily test JSweet (this will give us more feedback on potential language issues) (NOTE: this is ongoing work that should be out very soon)
• get better feedback from the community about using the plugins

Beside these aspects, another very important priority is the performance of using TypeScript, which can be improved as explained in issue #5.

All that being said, your use case might very well become more important in the close future, especially if we want to integrate to other tools... there are ongoing (so far private) discussions around this topic. The good news is that it is quite easy to refactor Java code to get a more programmer-friendly API :)

Thanks again for your feedback and stay tuned because a lot of things will happen in 2016!

[davidmoten]

Are we talking about code readability, code maintainability, code type safety, code safety, code reliability, etc...? When I read your feedback, it feels that your focus is on the API usability and how programmers shall use the JSweet transpiler APIs from Java.

Certainly quality is a vague term, I'll clarify.

I understand that the jsweet library is the core and its API will be consumed by tools (like eclipse/maven plugins etc) rather than normal users.

The comments about not using null are about API readability and clarity (because knowing that getXXX may return null (or Optional.empty()) can now be spelt out in a method signature instead of someone having to read javadoc or source code) and as a consequence can prevent NullPointerExceptions in code calling the API.

The comments about the use of immutable classes is a mixed bag too:

• thread safety 1 (visibility) - An instance of JSweetTranspiler may be referenced by an IDE plugin. When a user updates some option to do with transpilation in the UI and a setter is called then another part of the system that has been signalled to run the transpilation again may not see the change unless access to that setting has been synchronized.
• thread safety 2 (side-effects) - Again in an IDE if the setting has been changed and transpilation has been initiated and another change occurs then the change may have unexpected side effects on the first transpilation.
• thread safety 3 (no side-effects = can parallelize) - an immutable object for say JSweetTranspiler can perform transpilation in multiple threads simultaneously safely enabling parallel transpilation.
• immutability makes it easier to reason about a class. When setters (nullable ones) are available it is harder to reason about its state and lifecycle: Is the class currently in a state where it's ok to run transpilation? I'm currently at this point where when I look at JSweetTranspiler API or source code I don't know what the required fields are nor what the optional fields are.

Josh Bloch has a good discussion about the benefits of immutablility in Effective Java, Item 15 - Minimize mutability. To quote his advice:

Resist the urge to write a set method for every get method. Classes should be immutable unless there's a very good reason to make them mutable. Immutable classes provide many advantages and their only disadvantage is the potential for performance problems under certain circumstances.

By the way the performance problem referred to is due to object allocation overhead in circumstances not being encountered by JSweet.

That said I should say I'm impressed to see lots of unit tests run when I build the project and that's certainly one of the metrics I use to assess the quality of a project (as in will I use it).