Class that returns multiple unrelated values, are there better way to accomplish this?

Question

I have a "tracker" class. This class tracks objects from a given input image. But in addition to this, there is another parameter that corresponds to a value used in generating the image. The tracker may want to suggest a new value for this parameter to get better results on the next iteration. It would be suggesting this new value for what ever generates the image.

I currently implement this via:

struct TrackResults{
    double new_scene_generation_coefficient;
    std::vector<Track> tracks;
}

class Tracker{
public:
    TrackResults update(const Image& image, double scene_generation_coefficient){
        // generate tracks and a new_scene_generation_coefficient 
        // ...
        return {new_scene_generation_coefficient, tracks}
    }
}

My intuition is that we don't want to modify the value directly and we want to avoid having the Tracker depend on knowing about a class it only needs one value from, and could potentially change. I feel if I instead returned by reference here, I would potentially cause a cascading effect of anything that uses the tracker to depend on something it otherwise wouldn't need to, or pass around a mutable reference dangerously.

I don't find any issues in terms of maintainability with this, but there are some very slight ergonomic issues, for example, when I want tracks, I have to first go through TrackResult object.

I'm concerned however, since this seems like a code smell, the values in the data class have nothing to do with each other, and there are no methods that would use both in tandem. In other posts people argue that there should be no data classes, since you should just be able to move functions that work on the members of the class into a method.

Another thing I should mention, is that Tracks is maintained internally any way. Tracks are returned by value to avoid messing with the internal structure of the class from outside actors (like loggers which exist in the real system). This makes me think that tracks should be queried after update, and the new_scene_generation_coefficient is the only thing that should be returned. Potentially however, there are more values that would need to be used to edit the object which generated the Image in the first place, but at least these are related. An issue with the separate track query after update is that it becomes dangerous for the user, as there is no guarantee that they will try to query directly afterwards, and that even if they do, that they will get the state of the track list as it was when update() was called. This seems like a code smell to require a user to call another method first in order use a separate method in this way.

Answer 1

It looks like this can be cleaned up easily. You have a procedure that learns over iterations. Every time it is called, the quality of a parameter gets a little better. The first time that procedure is called you need some ballpark figure to start with.

So, you could pass initial_scene_generation_coefficient to the constructor of Tracker and store it as a member value scene_generation_coefficient. Remove the argument from Update's argument list and use the member instead, which you update on each call.

The generation coefficient is private to the Tracker, there is no reason to make it available outside of it apart from satisfying your curiosity (debugging).

You will want to lock the code in Update if you want to make it thread safe.

[Edit]

OK, I get it now after your comment. The mistake I see is that the tracker is supposed to have an understanding about image generation. That makes no sense and violates SRP. Tracker should just recognize objects in the image. There is no way it could assess the quality of the image, that requires either a human eye or a number of iterations that may yield a different number of recognized objects. If we rule out false positives, you could say the more recognized objects you get, the better the image quality must have been. It seems you want to feed the tracker a series of images generated with different lighting conditions and combine the tracking results. As you consistently get no results with some images, you may no longer bother to generate those (to use that generation coefficient because it is unrewarding). But the coefficient is meaningless in the context of the tracker so you would not pass it in or get it back.

You get back the recognized objects and you may try newly generated images to see if you get back more objects from the tracker. You could feed it both darker and lighter images and see if either yields more objects. That is the direction to move into with your coefficient until you get less objects again. Then you will have passed your optimum.

Answer 2

First of all I think the idea of "unrelated" here is a bit fuzzy if you need to constantly retrieve and pass that parameter back in to get the latest results. I don't know exactly what scene_generation_coefficient is supposed to do except that I'm seeing that you're basically making the client have to keep track of that state and constantly pass it into the object, get back a result, only to pass it back in again and repeat.

That's basically externalizing the responsibility of state management to the client (the person using your Tracker). And there are legit reasons to do that in some cases but I think most of the legit reasons tend to be in more procedural or functional contexts.

If we're utilizing OOP, then one of the key points of objects when it serves a practical need is to avoid this sort of thing. So one alternative design is to just make the Tracker store and update coefficient directly, and instantiate multiple trackers if you need to use these objects in disparate contexts. If you want to reuse the same tracker while completely disparate coefficients are being used simultaneously in different calling functions or threads at one time, then perhaps you might make TrackerResults responsible for that:

class TrackerResults
{
public:
    TrackerResults(...) {}

    // Even the image parameter might be a member passed through ctor
    // depending on design requirements.
    void update(const Image& image)
    {
        *this = tracker->update(image, scene_generation_coefficient);
    }

    const Track& operator[](size_t n) const 
    {
         return tracks[n];
    }

private:
    // Can use something like shared_ptr for robustness alternatively.
    Tracker* tracker;

    // Manage this internally in the object itself so the client doesn't
    // have to constantly retrieve and pass it back in.
    double scene_generation_coefficient;

    std::vector<Track> tracks;
};

... something like this for a crude illustration. Then the user just constructs one or more of these TrackerResults objects and calls update repeatedly to keep getting an improved result without having to manually manage this coefficients state. Something like this. If the client is free to ignore the "suggestion" for a new coefficient, then they might want to track the coefficient externally and you could program that feature into the TrackResults with some overloading, but mostly I'd try to make it so the client doesn't have to manage that state in a way that constantly requires passing in coefficient via parameter and capturing the resulting coefficient.

There's a balancing act here and it's not always so simple but a practical rule to start with before violating is don't make the client manage more state than they have to when it comes to OOP. You don't want to design a container class which requires the client to maintain the size of the container externally to the class itself, having to constantly pass it in and get back the new size in any function which can modify the size of the container as a very blatant and extreme example.

I can understand the concerns of avoiding side effects here but with objects a lot of the convenience comes from encapsulating the sort of state for which side effects are inevitable. To avoid overlap/overwrites and race conditions you end up using more than one object of that type in the same way you don't use one integer variable to capture every single integer output.

An issue with the separate track query after update is that it becomes dangerous for the user, as there is no guarantee that they will try to query directly afterwards, and that even if they do, that they will get the state of the track list as it was when update() was called.

I'll go ahead and list another concern in the same vein of thought which is that the shared state of the coefficient would make the tracker no longer thread-safe without locking. But that's a concern that's easily addressed by using multiple object instances with each instance maintaining its own separate coefficient. That's the OOP way where these types of functions do often incur a single side effect but the class makes it easy to maintain invariants and so forth. You could also throw a lock around this without modifying any client code if you need to utilize the same object as shared state for whatever reason.

I don't find any issues in terms of maintainability with this, but there are some very slight ergonomic issues, for example, when I want tracks, I have to first go through TrackResult object.

Blargh, ergonomics be damned in a language like C++. Trying to do things like minimize boilerplate too much is generally counter-productive here to favoring more straightforward code (if that's the alternative) that's less likely to be misunderstood. Still if we go with the proposed answers, then you get both a more ergonomic solution and one which shouldn't be too prone to misuse provided the client understands that the scene coefficient is part of the object's state being updated with each update call.