Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Override default methods in AbstractLwM2mRequest and ObserveCompositeRequest #1305

Closed
adamsero opened this issue Sep 6, 2022 · 3 comments
Closed
Labels
question Any question about leshan

Comments

@adamsero
Copy link
Contributor

adamsero commented Sep 6, 2022

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 sbernard31 added the question Any question about leshan label Sep 6, 2022
@sbernard31
Copy link
Contributor

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 🤔
I known at first side this sounds maybe obvious that we want to compare all field of the object, but here :

  • the coapRequest is not always here. (only here on incomming request)
  • when we will add new transport this could be a coapRequest or something like a kind of mqtt message

and so maybe we just want to compare the LWM2M part of the object (coapRequest is more like a kind optional transport layer data)

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

@adamsero
Copy link
Contributor Author

adamsero commented Sep 6, 2022

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
Copy link
Contributor

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Any question about leshan
Projects
None yet
Development

No branches or pull requests

2 participants