Use builder pattern for CoapRequest and CoapResponse #68
Conversation
|
@sbernard31 |
| public static Builder fetch(String uriPath) { | ||
| return request(Method.FETCH, uriPath); | ||
| } | ||
|
|
||
| public static CoapRequest fetch(String uriPath) { | ||
| return CoapRequest.of(null, Method.FETCH, uriPath); | ||
| public static Builder patch(String uriPath) { | ||
| return new Builder(Method.PATCH, uriPath); |
There was a problem hiding this comment.
Clearly a details but :
Some function use request() and other use new Builder I don't know if this is intended.
There was a problem hiding this comment.
Ohh, that's true, no that was not intended, will fix that
| public CoapRequest withToken(Opaque newToken) { | ||
| return new CoapRequest(method, newToken, options, payload, peerAddress, transContext); | ||
| } |
There was a problem hiding this comment.
I don't know if this kind of modifiers is a good idea 🤔, this looks like it does same job than builder ?
Maybe we should add a method which allow to initialize a builder with a coaprequest ?
e.g. :
new CoapRequest.Builder(coapRequest).token(t).build();
// OR maybe
coaprequest.modify().token(t).build(); // where modify(or a better name) return a builder ?There was a problem hiding this comment.
I agree, but I wanted to minimise need to change current code. Later we could add such a method and deprecate all with... functions
szysas
force-pushed
the
coap-request-response-builder
branch
from
6cb99d7
to
39184ea
Compare
|
Merging as there is no more comments. |
|
Uups, I'm not sure if you expected a confirmation from me ? Sorry about that. I haven't more comments to add, PR looks good to me. |
No description provided.