shamblett / coap

A Coap package for dart
Other
16 stars 13 forks source link

Slight tweak proposal to Uri handling #180

Closed JosefWN closed 5 months ago

JosefWN commented 1 year ago

Great work on passing the URI rather than constructing it internally (https://github.com/shamblett/coap/pull/169) @JKRhb.

Could it be nice to be able to write something like:

// Suggestion (naming of factory method could possibly also be shortened to just `get`)
CoapRequest.get(Uri(path: '/my/path', query: 'some_query=...'));
// Current, about 30% longer
CoapRequest.newGet(client.uri.replace(path: '/my/path', query: 'some_query=...'));

To me it would feel more intuitive and slightly more succinct?

To achieve that we could rename the client uri to baseUri and have a private function in the client:

Uri _buildUri(Uri uri) =>
  baseUri.replace(
    scheme: uri.scheme,
    userInfo: uri.userInfo,
    host: uri.host,
    port: uri.port,
    path: uri.path,
    pathSegments: uri.pathSegments,
    query: uri.query,
    queryParameters: uri.queryParameters,
    fragment: uri.fragment,
  );

... which we call in client._prepare or similar, I'm a bit rusty so don't know where the best place would be from the top of my head.

We could also potentially align the other methods with send a tiny bit by passing the uri there too, which would reduce the long argument list by one argument at least:

Future<CoapResponse> delete(
  final Uri uri, {
  final CoapMediaType? accept,
  final bool confirmable = true,
  final List<Option<Object?>>? options,
  final bool earlyBlock2Negotiation = false,
  final int maxRetransmit = 0,
  final CoapMulticastResponseHandler? onMulticastResponse,
})

Not sure about it yet, but I can write a PR if any of the suggestions sound good.

JKRhb commented 1 year ago

Thank you for the feedback, @JosefWN :) I actually had a similar thought and started working on something in #160, but I did not manage to finish the PR yet. In #160 I also tried to implement a more drastic API change, though, letting users set the URL for every request instead of the client, but that was a bit overkill (at least for the time being).

Therefore, I think your proposal looks pretty good! Let me know if you start implementing the changes, otherwise, I could also try to adapt #160 to a more manageable size in the spirit of your proposal :)