amino-os / Amino.Run

Amino Distributed OS - Runtime Manager
Apache License 2.0
29 stars 12 forks source link

Code Generator generates duplicated equals methods and hashcode method. #76

Open dhzhuo opened 6 years ago

dhzhuo commented 6 years ago

The current Stub Generator will insert its implementation of equals method and hashCode method in every _stub class

Meanwhile, the Stub Generator also creates a stub method for every public method in the base class.

Here is one problem. If the base class defines equals method and hashCode method. The generated stub class will have two equals methods and two hashCode methods - one set of methods are stub versions of the corresponding methods in the Base class, and the other set of methods are generated and inserted by Stub Generator (see here).

Java does not allow multiple instances of equals methods and hashCode method. So we will run into compilation error if our DM class contains its own equals and hashCode.

SrinivasChilveri commented 6 years ago

@DonghuiZhuo , May i know exact use case of this. I have gone thru the code & it just applicable for policy stubs in getStubAdditionalMethods() function. I have fixed to just over come the compilation issue (Just made it as other policy functions i.e. remote functions & removed the default ) but it has side effects , some where during toString the hashCode() function is invoked during SapphireObject creation but some info still not initialized which is required for the RPC like hostname etc. But i feel it may not be useful to implement the equal & hashCode functions in DM serverpolicy , group policies...what do you say?

The flags by default true but if the policy has these functions then i made these flags false, so that default code in stub will not be there. / Override equals / if (equalflag) { buffer.append(indenter.indent() + "@Override" + EOLN); buffer.append(indenter.indent() + "public boolean equals(Object obj) { " + EOLN); buffer.append( indenter.tIncrease() + stubName + " other = (" + stubName + ") obj;" + EOLN); buffer.append(indenter.tIncrease() + "if (! other.$oid.equals($oid))" + EOLN); buffer.append(indenter.tIncrease(2) + "return false;" + EOLN); buffer.append(indenter.tIncrease() + "return true;" + EOLN); buffer.append(indenter.indent() + "}" + EOLN); }

    /* Override hashCode */
    if (hashflag) {
        buffer.append(indenter.indent() + "@Override" + EOLN);
        buffer.append(indenter.indent() + "public int hashCode() { " + EOLN);
        buffer.append(indenter.tIncrease() + "return $__oid.getID();" + EOLN);
        buffer.append(indenter.indent() + "}" + EOLN);
    }
dhzhuo commented 6 years ago

How about we skip methods like equals and hashCode during stub generation?

quinton-hoole commented 6 years ago

Yes, the generator should not generate any methods for the stub itself that will also be present in object it is stubbing, IMO.

quinton-hoole commented 6 years ago

So that includes all methods of Object, at least:

https://docs.oracle.com/javase/7/docs/api/java/lang/Object.html

quinton-hoole commented 6 years ago

I don't think we need to solve this before release, right?

quinton-hoole commented 6 years ago

Removing label.

VenuReddy2103 commented 5 years ago

IMO, DM server policy and group policy objects do not need to implement equals() and hashCode() methods. Only generated server_stub and group_stub can implement them(like the way it is now). Because, server/group policy objects are not supposed to be injected into any dependency directly. Always stub objects are injected as dependency. For example, DM client has reference to server_stub and group_stub(but not to actual server/group object at any time). Similarly, server policy has reference to group_stub(but not to group object). And group policy has map of server_stub(but not that of server object).

IMO, we just document DM developer guide to not implement equals() and hashCode() methods. @quinton-hoole-2 What is your opinion ?

quinton-hoole commented 5 years ago

I'd have to look at the code more closely to comment. I don't have time to do that now. I don't think it's feasible to outlaw equals() and hashCode() (or any other methods of Object, as mentioned in my comment above). What if a user wants to put stubs in a hash table or set? Unlikely, but I'm sure it will bite us eventually. Better to fix it properly by simply not generating duplicate methods in the stubs.