eclipse-leshan / leshan

Java Library for LWM2M
https://www.eclipse.org/leshan/
BSD 3-Clause "New" or "Revised" License
653 stars 407 forks source link

Override default methods in AbstractLwM2mRequest and ObserveCompositeRequest #1305

Closed adamsero closed 2 years ago

adamsero commented 2 years ago

Since these classes don't have their custom implementations of toString(), hashCode() and equals, I created them and pushed them here. I tested them against common use cases and they seem to work alright. Let me know if there's any issues.

sbernard31 commented 2 years ago

I guess for ObserveCompositeRequest, it makes sense.

For AbstractLwM2mRequest, it's less clear because I don't know if we really want to compare the coapRequest object :thinking: I known at first side this sounds maybe obvious that we want to compare all field of the object, but here :

adamsero commented 2 years ago

To be fair, I implemented the two methods in AbstractLwM2mRequest because I noticed a practice of calling super methods from subclasses, just like hashCode() and equals() in ObserveRequest calls the respective super methods in AbstractSimpleDownlinkRequest.

Do you face a case where you need to compare the coapRequest part ?

I can't really answer that question as I'm rarely in contact with actual practical use of Leshan, but at the very least I'd like to have the overriding methods in ObserveCompositeRequest, I only cared about AbstractLwM2mRequest for integrity's sake.

sbernard31 commented 2 years ago

because I noticed a practice of calling super methods from subclasses, just like hashCode() and equals() in ObserveRequest calls the respective super methods in AbstractSimpleDownlinkRequest.

Yep, Theoretically you should not find request which call AbstractLwM2mRequest.equals or AbstractLwM2mRequest.hashcode, if you find it looks like a bug. (but I agree that current code need more documentaiton about that or more better design for this part of the code)

I can't really answer that question as I'm rarely in contact with actual practical use of Leshan,

Too bad.

but at the very least I'd like to have the overriding methods in ObserveCompositeRequest, I only cared about AbstractLwM2mRequest for integrity's sake.

Yep let's go first with ObserveCompositeRequest (then we will see later about how to handle AbstractLwM2mRequest case better)