Details

    • Type: New Feature New Feature
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.0
    • Fix Version/s: 1.0.4
    • Component/s: XML code generator
    • Labels:
      None
    • Environment:
      Windows XP SP2, Java JDK 1.5.0_03, Castor 1.0
    • Number of attachments :
      4

      Description

      Allows the source generator tool to create the hashCode() method when the equals() method is created.

      1. c1417-final.diff
        9 kB
        Edward Kuns
      2. c1417-patch.diff
        7 kB
        Edward Kuns
      3. patch-C1417-20060602.txt
        5 kB
        Ralf Joachim
      4. SourceFactory.java
        86 kB
        Michele Mazzucco

        Activity

        Hide
        Ralf Joachim added a comment -

        Thanks again Michele, I'll look into it later on.

        Show
        Ralf Joachim added a comment - Thanks again Michele, I'll look into it later on.
        Hide
        Ralf Joachim added a comment -

        Final patch

        Show
        Ralf Joachim added a comment - Final patch
        Hide
        Michele Mazzucco added a comment -

        Ralf, I see that there is still a little detail to fix (however, the hashCode() contract is respected): Castor creates some boolean fields (i.e. has...) and the code generator considers those fields when it creates the hash code. It would be much better to avoid to include them.

        Show
        Michele Mazzucco added a comment - Ralf, I see that there is still a little detail to fix (however, the hashCode() contract is respected): Castor creates some boolean fields (i.e. has ...) and the code generator considers those fields when it creates the hash code. It would be much better to avoid to include them.
        Hide
        Ralf Joachim added a comment -

        Generation of the hashCode() method causes 4 tests of XML MasterTestSuite to fail. I will back out the patch until this problem is resolved and move the issue over to next release.

        Show
        Ralf Joachim added a comment - Generation of the hashCode() method causes 4 tests of XML MasterTestSuite to fail. I will back out the patch until this problem is resolved and move the issue over to next release.
        Hide
        Ralf Joachim added a comment -

        One idea to resolve the problem could be to make generation of hashCode() method configurable by properties file or command line option.

        Michele could you please prepare a new patch with your suggested additions. Maybe you can run XML test suite also at your side and try to find the problem with the failing tests.

        Show
        Ralf Joachim added a comment - One idea to resolve the problem could be to make generation of hashCode() method configurable by properties file or command line option. Michele could you please prepare a new patch with your suggested additions. Maybe you can run XML test suite also at your side and try to find the problem with the failing tests.
        Hide
        Michele Mazzucco added a comment -

        Ralf, the hashCode() generation is already controlled by the property org.exolab.castor.builder.equalsmethod in castorbuilder.properties.
        This is because you must override the hashCode() method every time you override the equals() method, otherwise you can expect unpredictable results. Furthermore, sometimes the hashCode() method is used to control if two object are equals (see the collection API, for example).

        Show
        Michele Mazzucco added a comment - Ralf, the hashCode() generation is already controlled by the property org.exolab.castor.builder.equalsmethod in castorbuilder.properties. This is because you must override the hashCode() method every time you override the equals() method, otherwise you can expect unpredictable results. Furthermore, sometimes the hashCode() method is used to control if two object are equals (see the collection API, for example).
        Hide
        Michele Mazzucco added a comment -

        I see the _has*** fields are added into the FieldInfo class. My idea would be to track somewhere (SourceFactory???) which "false" fields are created and then avoid to use them during the generation of the equals/hashCode/toString methods.

        Show
        Michele Mazzucco added a comment - I see the _has*** fields are added into the FieldInfo class. My idea would be to track somewhere (SourceFactory???) which "false" fields are created and then avoid to use them during the generation of the equals/hashCode/toString methods.
        Hide
        Ralf Joachim added a comment -

        Michele, I am familar with the design contract of equals() and hashCode() methods. This was the reason why I catches up this issue and tried to resolve it. The problem is that the hashCode() method created with your patch causes 4 test cases of the XML test suite to fail. As some of Castor's useres may rely on the current behaviour these tests cover I can not change that without knowing what's happening.

        I therefore gave it a second try and searched for the reason of the problem for quite some time without success. I even could not find a possibility to debug through the failing tests. Having said that the failure message says that 2 of them fail according to a file that could not be found. This does not make any sense to me and I desided to give up with this as long as no one is able to give me usefull hints to resolve the problems.

        Show
        Ralf Joachim added a comment - Michele, I am familar with the design contract of equals() and hashCode() methods. This was the reason why I catches up this issue and tried to resolve it. The problem is that the hashCode() method created with your patch causes 4 test cases of the XML test suite to fail. As some of Castor's useres may rely on the current behaviour these tests cover I can not change that without knowing what's happening. I therefore gave it a second try and searched for the reason of the problem for quite some time without success. I even could not find a possibility to debug through the failing tests. Having said that the failure message says that 2 of them fail according to a file that could not be found. This does not make any sense to me and I desided to give up with this as long as no one is able to give me usefull hints to resolve the problems.
        Hide
        Werner Guttmann added a comment -

        Ralf, I know it's frustrating to see such a simple patch cause problems with the test suite of Castor XML. Before giving up, let's try to get answers from the folks that should now. Why not sending an email to either committers or folks directly, asking them to review the patch and (potentially) comment upon the four tests failing.

        Show
        Werner Guttmann added a comment - Ralf, I know it's frustrating to see such a simple patch cause problems with the test suite of Castor XML. Before giving up, let's try to get answers from the folks that should now. Why not sending an email to either committers or folks directly, asking them to review the patch and (potentially) comment upon the four tests failing.
        Hide
        Michele Mazzucco added a comment -

        Ralf, this makes no sense since I don't make any I/O access (unless a field into the XML schema maps to a file). Could you please give me any hint to run the XML suite, since I get some errors when trying to run the all suite?

        Show
        Michele Mazzucco added a comment - Ralf, this makes no sense since I don't make any I/O access (unless a field into the XML schema maps to a file). Could you please give me any hint to run the XML suite, since I get some errors when trying to run the all suite?
        Hide
        Ralf Joachim added a comment -

        Werner, I'll send a mail to Keith if he is in a position to review patch and comment on the failures.

        Michele, to run the XML test suite you have to check out a fresh copy of Castor from SVN or download the source distribution of castor. Be aware that some resource files are missing at 1.0.1 source distribution. Therfore I suggest to use SVN or if that is not possible for you the 1.0 source distribution. Now the XML test suite can be executed as follows:

        CD /castor/bin
        build clean
        build tests
        CTFRun

        Without the hashCode() patch all tests sould pass except of one that randomly fails. If you apply the patch you will see 4 tests of MasterTestsuite in the sourcegenerator part failing (in addition to the one that fails by random).

        Hope this helps.

        Show
        Ralf Joachim added a comment - Werner, I'll send a mail to Keith if he is in a position to review patch and comment on the failures. Michele, to run the XML test suite you have to check out a fresh copy of Castor from SVN or download the source distribution of castor. Be aware that some resource files are missing at 1.0.1 source distribution. Therfore I suggest to use SVN or if that is not possible for you the 1.0 source distribution. Now the XML test suite can be executed as follows: CD /castor/bin build clean build tests CTFRun Without the hashCode() patch all tests sould pass except of one that randomly fails. If you apply the patch you will see 4 tests of MasterTestsuite in the sourcegenerator part failing (in addition to the one that fails by random). Hope this helps.
        Hide
        Keith Visco added a comment -

        I'll see what I can find out...I need to set up a new build environment first however... so give me a day to do that and see if I can spot the issues with the test cases.

        Show
        Keith Visco added a comment - I'll see what I can find out...I need to set up a new build environment first however... so give me a day to do that and see if I can spot the issues with the test cases.
        Hide
        Edward Kuns added a comment -

        I'll take a look at this.

        Show
        Edward Kuns added a comment - I'll take a look at this.
        Hide
        Edward Kuns added a comment -

        I can reproduce the test case failures with this patch applied. Something is bizarre!

        Show
        Edward Kuns added a comment - I can reproduce the test case failures with this patch applied. Something is bizarre!
        Hide
        Edward Kuns added a comment -

        There were two problems with hashCode(). One was that an object with multiple double variables would have multiple definitions of "long tmp". The other is that null objects caused the hashCode() recursion to return a null pointer exception. I'll shortly attach a new patch that is ready for review.

        Show
        Edward Kuns added a comment - There were two problems with hashCode(). One was that an object with multiple double variables would have multiple definitions of "long tmp". The other is that null objects caused the hashCode() recursion to return a null pointer exception. I'll shortly attach a new patch that is ready for review.
        Hide
        Edward Kuns added a comment -

        Attached is a new patch against trunk that works for me and that fixes the problems noted above. This patch also correctly does not include the has* variables in the hashCode() calculation. But only if it's the has* that was generated by Castor for a Primitive.

        Show
        Edward Kuns added a comment - Attached is a new patch against trunk that works for me and that fixes the problems noted above. This patch also correctly does not include the has * variables in the hashCode() calculation. But only if it's the has * that was generated by Castor for a Primitive.
        Hide
        Werner Guttmann added a comment -

        Great. I think Ralf will be quite reliefed to se this fixed finally ....

        Show
        Werner Guttmann added a comment - Great. I think Ralf will be quite reliefed to se this fixed finally ....
        Hide
        Edward Kuns added a comment -

        Final patch, including release notes.

        Show
        Edward Kuns added a comment - Final patch, including release notes.

          People

          • Assignee:
            Edward Kuns
            Reporter:
            Michele Mazzucco
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: