open-coap / java-coap

CoAP Java library
Apache License 2.0
18 stars 5 forks source link

GH-73 Add context modifier to CoapRequest #74

Closed szysas closed 7 months ago

szysas commented 7 months ago

@sbernard31 would that work for you? Very simple addition to support context modifier.

sbernard31 commented 7 months ago

It could do the job but during this conversation (https://github.com/open-coap/java-coap/pull/68#discussion_r1344480669), I understood that with*** method was mainly temporary solution.

So maybe we can directly go with a more future proof changes ? Like :

coapRequest.modify()
           .context(ctx -> ctx.with(key,value))
           .build();

Where modify will be :

public final class CoapRequest {
  // ...

  public CoapRequest.Builer modify() {
    return new CoapRequest.Builer(this);
  }

  // ...

  public static class Builder {
    private Builder(CoapRequest coapRequest) {
            this.method = coapRequest.method;
            this.options = CoapOptionsBuilder.from(coapRequest.options);
            ... 
            ...
            this. transContext = coapRequest.transContext; 
    }
  }
}

I mean we add this modify method but you keep the existing with** methods and you will deprecated/remove it when you think this will be the right moment.

szysas commented 7 months ago

You're right, going directly to .modify() function is a better option. How about now?

sbernard31 commented 7 months ago

Yep it sounds good. :+1:

So to add a entry in transport context of a request, this will look like :

coapRequest.modify()
           .context(key,value)
           .build();

Do we need to add this methods to builder ?

public Builder context(Consumer<TransportContext> contextFunc) {
    contextFunc.apply(transport);
    return this;
}

That was my initial idea but thinking a bit more about it I think this is not really needed. Any opinion ?

szysas commented 7 months ago

I don't think it is needed, anyway we can always add it later.