okleine / nCoAP

Java implementation of the CoAP protocol using netty
BSD 3-Clause "New" or "Revised" License
179 stars 57 forks source link

Do not register a service twice. #27

Closed boldt closed 9 years ago

boldt commented 9 years ago

It was possible to register the same resoruce twice. This is fixed and a WARN log is written.

okleine commented 9 years ago

Thank you for bringing this up. However, I think it is worth to consider some alternatives on how to deal with this. Just to log a warning is not of much help from an applications perspective that registered two services at the same path by accident. Maybe one could

a) make the method "registerService" return true or false, or b) path-through the result of the internal put operation, i.e. either "null" or the Webservice instance that was registered at the same path beforehand, or c) keep it a void method and replace the previosuly registered service but additionally invoke the shoutdown method on the replaced Webservice.

Any preferences?

danbim commented 9 years ago

IMHO this is a perfect situation for throwing an (IllegalState?)Exception (i.e. a non-checked runtime exception). This would clearly give a hint to the app programmer that his code is flawed. If he really intends to he should check first for an existing service, maybe remove it and then replace with the new.

Or alternatively.... if you want it to be an somewhat more atomic operation... make the operation "idempotent", i.e. replace the existing (Option b) or c) (?)).

boldt commented 9 years ago

We discussed that problem offline as well. We also came to the point, that an exception would be another good option.

okleine commented 9 years ago

Alright, RuntimeException it is... I think, the IllegalArgumentException is most appropriate without the need to define a new subclass.