11 Replies Latest reply on Nov 19, 2015 8:44 AM by tomjenkinson

    Adding generics

    pmm

      Narayana/JBossTM currently still used raw types in a lot places. Most of them can be fixed easily.

       

      Here's a PR with the proposed changes:

      https://github.com/jbosstm/narayana/pull/943/files

      this also includes changes to some public API. Most notably:

      • com.arjuna.ats.arjuna.coordinator.RecordType.typeToClass(int)
      • com.arjuna.ats.arjuna.coordinator.abstractrecord.RecordTypeManager.getType(Class<? extends AbstractRecord>)

       

      I did not make com.arjuna.ats.arjuna.coordinator.RecordType.classToType(Class) symmetric with typeToClass(int) because then com.hp.mwtests.ts.arjuna.common.TypesUnitTest.testRecordType() would no longer compile.

       

      Due to the design of generics I believe this change is both source and binary compatible. Binary compatible because of type erasure. Source compatible because source using raw types should still compile and so should source using generics (if it uses the correct type parameters).

       

      I could also split the commit into two, one for just the internal changes and one for the API changes.

        • 1. Re: Adding generics
          mmusgrov

          Hi, thank you for the updates and they do make the code look and read cleaner. However changes to the code base are normally to fix bugs or add features or improve performance and the PR job output shows a neutral impact on performance. Please could you provide more explanation of why this change is necessary.

          • 2. Re: Adding generics
            tomjenkinson

            Michael Musgrove wrote:

             

            Hi, thank you for the updates and they do make the code look and read cleaner. However changes to the code base are normally to fix bugs or add features or improve performance and the PR job output shows a neutral impact on performance. Please could you provide more explanation of why this change is necessary.

            +1, lets understand the motivation for the change first. We can then look at internal vs external code API changes given the assertion it would still be source compatible.

            • 3. Re: Adding generics
              marklittle

              +1

               

              We also tend to be cautious about new language features because some in the community still use *really old* versions of Java What's the minimum version we support these days and do we know if that's just because we don't build ourselves on older versions or because we've added support for language features which aren't in older versions?

              • 4. Re: Adding generics
                tomjenkinson

                We currently build with both JDK7 and JDK8. I have used animal-sniffer to work out whether we can build with older JDKs but JDK7 is the oldest we can build with as we started using suppressedThrowables a while back (for a usability improvement).

                • 5. Re: Adding generics
                  tomjenkinson

                  tomjenkinson wrote:

                   

                  We currently build with both JDK7 and JDK8. I have used animal-sniffer to work out whether we can build with older JDKs but JDK7 is the oldest we can build with as we started using suppressedThrowables a while back (for a usability improvement).

                   

                  If anyone ends up back here in future and wants to retest:

                   

                      <plugin>

                        <groupId>org.codehaus.mojo</groupId>

                        <artifactId>animal-sniffer-maven-plugin</artifactId>

                        <version>1.14</version>

                        <executions>

                          <execution>

                            <id>check-java-version</id>

                            <phase>compile</phase>

                            <goals>

                              <goal>check</goal>

                            </goals>

                            <configuration>

                            <signature>

                                <groupId>org.codehaus.mojo.signature</groupId>

                                <artifactId>java17</artifactId>

                                <version>1.0</version>

                            </signature>

                            </configuration>

                          </execution>

                        </executions>

                      </plugin>

                  • 6. Re: Adding generics
                    pmm

                    Michael Musgrove wrote:

                     

                    Hi, thank you for the updates and they do make the code look and read cleaner. However changes to the code base are normally to fix bugs or add features or improve performance and the PR job output shows a neutral impact on performance. Please could you provide more explanation of why this change is necessary.

                    Several reasons:

                    1. Consistency with the rest of the code base. Most of the other code already uses generics. For example typed ArrayLists are already used in ten classes in arjuna module alone. Generics seem to be the rule rather than the exception in the Narajana codebase. What's left without generics more seems like left overs that fell through the cracks.
                    2. Generics are generally considered a "good" feature. They add additional type checks at compile time. This provides additional safety and often removes the need for casts. That's probably why they are used in so many other places in the Narajana codebause.
                    3. Codebases without generics have a certain "legacy" feel to them that may make it harder to attract community contributors.
                    • 7. Re: Adding generics
                      mmusgrov

                      Philippe Marschall wrote:

                       

                      1. Consistency with the rest of the code base. Most of the other code already uses generics. For example typed ArrayLists are already used in ten classes in arjuna module alone. Generics seem to be the rule rather than the exception in the Narajana codebase. What's left without generics more seems like left overs that fell through the cracks.

                      After generics was added to the language any new code we add makes use of them. We decided against refactoring existing code to avoid risk and to make it easier to diff changes. Another decision we made was around style:- when modifying existing code we generally stick to the style used in the file we are changing.

                      1. Generics are generally considered a "good" feature. They add additional type checks at compile time. This provides additional safety and often removes the need for casts. That's probably why they are used in so many other places in the Narajana codebause.

                      Agreed but since we have not modified code added pre Java 5 we are probably safe with respect to the additional type safety that generics provides.

                      Codebases without generics have a certain "legacy" feel to them that may make it harder to attract community contributors.

                      Agreed but I think the reasons I have just given outweigh this disadvantage. Besides I'd like to think that our legacy is that we provide one of the best transaction implementations in the industry.

                      • 8. Re: Adding generics
                        tomjenkinson

                        Michael Musgrove wrote:

                         

                        I'd like to think that our legacy is that we provide one of the best transaction implementations in the industry.

                         

                        +1

                         

                        @philippe - many thanks for the offer of the contribution. There are a lot of features in our issue tracker (JBoss Transaction Manager - JBoss Issue Tracker) that it would be great to get your input on. If you would like to discuss which ones might be suitable you could chat with us on freenode Web IRC (qwebirc) and we can see which would match your interests.

                        • 9. Re: Adding generics
                          marklittle

                          It's also worth emphasising that some changes we could add based on new features in the language over the years would necessitate changes to public APIs and we try really hard not to break compatibility without good reasons. As Mike has already mentioned, performance is one of those reasons where we will make these kinds of changes, but it has to be a significant improvement to warrant.

                          • 10. Re: Adding generics
                            pmm

                            So, should I close the PR and the JIRA?

                            • 11. Re: Adding generics
                              tomjenkinson

                              Hi Philippe,

                               

                              I will close them with the right status'.

                               

                              I do hope we can work with you in the future - I appreciate the effort you have put in and hopefully we can find an issue that interests you and is in line with our roadmap.

                               

                              Thanks again,

                              Tom