ResourceRetriever and Resource methods are now const

[mkoval]

It looks like #517 const-qualified all of the methods on the ResourceRetriever and Resource classes. I intentionally left these non-const when I added these classes in #464 so the user can implement a cache in his or her custom implementations. This is important when retrieving resources over the network or when resolving the URI is computationally expensive.

I just ran into this situation myself and had to make a bunch of my member variables mutable as a workaround. I don't see any advantage to making these methods const, so I would prefer to remove the const-qualifiers added in #517.

[jslee02]

I though the methods, exists and retrieve, should be const-qualified by the meaning of them, and we generally don't expect that getter functions modify the member variables. As I known, using mutable members for caching data (required expensive computation) is well known technique called logical constness. Also, we already use it in the auto-updating kinematics mechanism.

Above is my understand so far, and I would prefer to keep the consistency of the use of cost correctness. However, I'm not sure which one is practically preferred way.

[mkoval]

I don't think exists and retrieve are logically const.

ResourceRetrievers operate on some sort of remote data store, e.g. the filesystem, the network, or a database. Even if our implementation of those methods are logically const, there is no way we can guarantee that the remote operation is logically const. For example, making an HTTP GET request could modify state on the server. The caller can't distinguish between mutation of state in the ResourceRetriever and this type of mutation of state on the remote server.

@mxgrey @psigen Thoughts?

[psigen]

I don't think that const qualification really affects @mkoval's case.

Here, the const qualifier is indicating that nothing in our data structure is being mutated by the exists or retrieve call, which is about all we could ever guarantee. Nothing about our internal logic or state will change as a result of these calls.

We can never make any guarantees about remote calls. Making the same remote call twice might yield different results just because the file on disk changed, or the server drops an HTTP call, or any number of arbitrary conditions unrelated to our code, but I don't think that violates the const nature.

As long as we do not change the internal state of our program as a result, and we will follow the exact same logic when we make that call again, I think we can call these const. There are plenty of examples of const methods that have logically non-const effects on other things.

[mxgrey]

I think there's a compelling enough case for them to not be const. I suppose the big question would boil down to: What would be the meaning of a const ResourceRetriever? What would you expect to be able to do and to not do with a const ResourceRetriever?

So far our approach to const-correctness has been "as long as every function you call is a const-qualified function, every result that you get from any given function should be the same each time you call it with the same input arguments". By this definition, you might reasonably expect exists and retrieve to be candidates for constness because you would think that asking for the same resource over and over again would produce the same result. But if you consider that most ResourceRetrievers are interacting with external resources that you don't necessarily have control over, there can't really be an expectation that you will get the same result every time.

My conclusion would be that, even though a user might reasonably expect that exists and retrieve would be logically const, we can't guarantee that. And since const qualification is meant to be a guarantee, we should not const-qualify these functions.

[psigen]

Hmm, I guess @mxgrey and I are just on different sides of the argument: "Is it reasonable to expect a const qualified function to have to return the same result every time?"

On my side, I think that it is OK that it does not, as long as it uses the same exact deterministic logic to generate the result each time, and the C++ object does not change state. But Grey thinks that the existing const convention in DART is that the result itself should be deterministic.

If there is such a convention already, then we should go with that.

[mkoval]

I generally agree with @mxgrey here.

As a higher level point, remember that ResourceRetriever is a pure-virtual interface that we intend the user to inherit from. Marking exists and retrieve as const-qualified means that anyone who implements a ResourceRetriever class must not have any internal state in their C++ object (even under @psigen's relaxed criteria).

I'm not comfortable making that commitment for all possible implementations of the interface.

[psigen]

Ok, for a virtual interface class it does seem like const is way too strict, I am on the same side as @mkoval and @mxgrey now (why do I ever disagree with them? 😉).

[jslee02]

I now agree with that const qualifier does not fit for exists and retrieve because of the fact that we cannot guarantee the result is the same per same argument. Also, it would be good to leave some comment on those functions about the reason why we didn't const qualify for them so that someone (like me) would not try to change back. 😜

[psigen]

Can we close this now?