Client.OptionsReader() returns a struct ClientOptionsReader instead of interface which make mocking impossible

[avmunm]

Hi,

Client.OptionsReader returns this object.

// ClientOptionsReader provides an interface for reading ClientOptions after the client has been initialized.
type ClientOptionsReader struct {
    options *ClientOptions
}

When trying to mock the Client and using the options reader in my applications I get a nilpointer, as I can only return an empty struct of ClientOptionsReader. I think it would be better if ClientOptionsReader was an interface, so I could also mock it. Unless I am missing something else I could do? Going from this:

func NewMockClient() mqtt.Client {
    return &mockMQTTClient{}
}

type mockMQTTClient struct {
    subscriptions map[string]byte
}

func (c *mockMQTTClient) OptionsReader() mqtt.ClientOptionsReader {
    return mqtt.ClientOptionsReader{}
}

To this:

func NewMockClient() mqtt.Client {
    return &mockMQTTClient{}
}

type mockMQTTClient struct {}

func (c *mockMQTTClient) OptionsReader() mqtt.ClientOptionsReader {
    return &mockOptionsReader
}

type mockOptionsReader struct {}

func (r *mockOptionsReader) ResumeSubs() bool {
    return false
}
....

[MattBrittan]

I've never had the need to use OptionsReader() so this is not something I'd considered and can see your point (there is no way for you to create a valid ClientOptionsReader).

I can see two ways of resolving this:

• Your suggestion; change the signature of Client such that ClientOptionsReader is an interface.
• Add a func NewOptionsReader(o *ClientOptions) ClientOptionsReader to the library.

I agree that your solution is the cleanest, however it's also potentially a breaking change, so the second method might be simpler (provides the functionality you need without any potential to break others code).

While the first change is potentially breaking I doubt that many (any?) users are actually using this in a way that would be broken so am open to being convinced that this is the right way to go (but really don't want to have to release a v2 of this library!).

[avmunm]

Thanks for the fast response! If i find some free time I can try to implement a solution and make a PR. We can see it more clearly once it's implemented I think.

[MattBrittan]

OK - which method where you thinking of going with? (I'd accept a PR utilising the second option without much thought; if you change the ClientOptionsReader then that would require more consideration because it's technically a breaking change).

[avmunm]

I'd chose the second option if you think that's less risky then. My proposal came from a perspective of what it'd do in my own code base I haven't done any real Library work.