Clips 6.4.1 JNI Primitive value HashCode and Equals

[Chaubert Jérôme]

Hi,

In Java class "PrimitiveValue" with Clips JNI 6.31. I have the following hashCode and equals (I honestly don't remember if it is my code or yours) :

   /*************/
   /* hashCode: */
   /*************/
   @Override
   public int hashCode()
     {
      final int prime = 31;
      int result = 17;
      result = prime * result + ((this.value == null) ? 
                                 0 : this.value.hashCode());
      return result;
     }

   /***********/
   /* equals: */
   /***********/
    @Override
    public boolean equals(Object obj) 
      {
       if (this == obj) return true;
       if (obj == null) return false;
       if (this.getClass() != obj.getClass()) return false;
       PrimitiveValue pv = (PrimitiveValue) obj;
       if (this.value == null) return (pv.value == null);
       return this.value.equals(pv.value);
      }

But with Clips 6.4.1 I have these methods :

   /*************/
   /* hashCode: */
   /*************/
   @Override
   public int hashCode()
     {
      return 0;
     }

   /***********/
   /* equals: */
   /***********/
   @Override
   public boolean equals(Object obj) 
     {
      if (this == obj) return true;
      if (obj == null) return false;
      if (this.getClass() != obj.getClass()) return false;
      return true;
     }

The first version make more sense to me. With the second version (6.4.1) two different facts will be equals (for exemple). Is it an error or did you do this on purpose ?

[Gary Riley]

I changed the code so that the subclasses of PrimitiveValue override the base methods for equal and hashCode, but it looks like I forgot to do this for the FactAddress, InstanceAddress, and ExternalAddress subclasses. I checked in some code to the svn repository to correct this.

[Chaubert Jérôme]

Perfect. Thank you. Can you tell me when it's done (I prefer to have as little difference as possible between our codes).

[Gary Riley]

It's been checked in already: https://sourceforge.net/p/clipsrules/code/884/

[Chaubert Jérôme]

Sorry to react so late, I had my own fix and was working on other points. I think there's still a problem with the equals method because you're using "==" on objects (e.g. Long) (which doesn't check the value of the object but the address). You should use ".equals" to get the desired behavior. Furthermore, in this case, it seems to me that the method definition could be in the parent class (PrimitiveValue) as its code doesn't change in any of it's subclasses :

/*************/
  /* hashCode: */
  /*************/
  @Override
  public int hashCode()
  {
      final int prime = 31;
      int result = 17;
      result = prime * result + ((this.getValue() == null) ?
              0 : this.getValue().hashCode());
      return result;
  }

  /***********/
  /* equals: */
  /***********/
  @Override
  public boolean equals(Object obj)
  {
      if (this == obj) return true;
      if (obj == null) return false;
      if (this.getClass() != obj.getClass()) return false;
      PrimitiveValue pv = (PrimitiveValue) obj;
      if (this.getValue() == null) return (pv.getValue() == null);
      return this.getValue().equals(pv.getValue());
  }

[Gary Riley]

I don't recall why I moved the equals code from the superclass to the subclasses, but use of == in the equals method is just applying reflexivity. An object is equal to itself, so if both objects compared have the same address, they're equal. Otherwise , the values of the object need to be compared. This was the most common design pattern I found when searching for implementing the equals method.

[Chaubert Jérôme]

Okay, but the behavior isn't what I expected.

With the following Clips code (in a file "templates.clp") :

(defmodule MAIN)

(deftemplate adresse
    (slot nom                         (type STRING)(default ""))
    (slot pays                        (type INTEGER)(default 0)))

(deffunction init ()
  (bind ?adresse1 (assert (adresse (nom "John")(pays 1)))))

and the following Java code :

@Test
public void readTemplates() throws Exception {
    final Environment clips = new Environment();
    final String clp = "./src/test/resources/templates.clp";
    clips.load(ClipsFilePathConverter.getClipsPath(new File(clp)));
    clips.eval("(init)");
    MultifieldValue facts=(MultifieldValue) clips.eval("(get-fact-list *)");
    FactAddressValue f1=(FactAddressValue) facts.getValue().get(0);
    MultifieldValue factsToo=(MultifieldValue) clips.eval("(get-fact-list *)");
    FactAddressValue f1Too=(FactAddressValue)  factsToo.getValue().get(0);
    if(f1.equals(f1Too)) {// <---- I think it should be true
        System.out.println("f1 and f1Too are equals");
    }else{
        System.out.println("f1 and f1Too are not equals. Details :");
        System.out.println("f1.getValue() : "+f1.getValue());
        System.out.println("f1Too.getValue() : "+f1Too.getValue());
        boolean witheqeq=f1.getValue()==f1Too.getValue();
        System.out.println("f1.getValue()==f1Too.getValue() : "+witheqeq);
        boolean withequals=f1.getValue().equals(f1Too.getValue());
        System.out.println("f1.getValue().equals(f1Too.getValue()) : "+withequals);
    }
}

I get this print result:

f1 and f1Too are not equals. Details :
f1.getValue() : 2281038163056
f1Too.getValue() : 2281038163056
f1.getValue()==f1Too.getValue() : false
f1.getValue().equals(f1Too.getValue()) : true

I expected an equality between facts "f1" and "f1Too". (With ".equals" instead of "==" in "equals" method it works as I expect).

[Gary Riley]

OK, I'll check that out.

[Gary Riley]

OK, I confused myself and thought the == you were referring to was in the code posted beneath your comments instead of the equals in the FactAddressValue class. Your fix corrects the issue. I'll check in fix for that shortly. I also was able to move a lot of the code back into the PrimitiveValue class. I think the reason I might have kept that code in the subclasses was because I was getting an unchecked cast warning from the MultifieldValue class when I cast the value from the PrimitiveValue superclass to a List<primitivevalue>, but everything seems to work correctly if I just store the value privately in the MultifieldValue class and override the equals method for just that class.</primitivevalue>

[Gary Riley]

I checked in a fix.

[Chaubert Jérôme]

Thanks. It's perfect like that.