eclipse/paho.golang

ClientConfig: Inconsistent mix of exported and private (with setter) fields

Closed this issue · 2 comments

MattBrittan commented

Autopaho.ClientConfig originally contained exported fields only; PR #69 added a range of un-exported fields with setter functions e.g.

connectUsername string
connectPassword []byte

willTopic           string
willPayload         []byte
...
connectPacketBuilder func(*paho.Connect) *paho.Connect

and a few further examples have been added since (where the settings were similar to those already in place).

We really should standardise on one approach before releasing V1. I'm suggesting that all fields be exported because:

  • ClientConfig does not contain a Mutex, and a copy is passed to NewConnection, so races should not be an issue (it's clear that the config should not be changed after calling NewConnection).
  • Consistency with paho.ClientConfig
  • Going this way is non-breaking (the existing setters can be left and depreciated).

At the same time I think it's worth renaming BrokerUrls to ServerUrls because the MQTT spec does not use the term Broker so its likely to be confusing to those new to the protocol. Again this can be non-breaking (NewConnection can overwrite ServerUrls with BrokerUrls if its empty).

alsm commented

I prefer to make things exported as default, as you say with there being no mutex an the struct being copied it should be safe/fine to move to exported fields for the autopaho config.

XANi commented

@MattBrittan just wanted to let you know that you accidentally fixed a bug here; before setting will with empty payload (example use case: using will to clear retained message when client disconnect) did not set the will on connect (because condition was if len(cfg.willTopic) > 0 && len(cfg.willPayload) > 0 { , now it works correctly.

Thanks :)

  • 👍1