[Development] API for multiple (but variable number of) return arguments

[Andre Somers]

Hi Sze-Howe,

Thank you very much for your input.

Op 28-7-2013 6:10, Sze Howe Koh schreef:
> On 27 July 2013 17:36, Andre Somers <andre at familiesomers.nl> wrote:
>> 1) pointers to ints
> Agree that it's not very nice. I'd imagine that in many cases, the
> developer is only interested in the minority of the units.
>
>
>> 2) returning a map
>> QMap<Qt::TimeSpanUnit, int> getUnits(Qt::TimeSpanFormat);
> <snip>
>
>> In my opinion, not all that great either, but better than 1). It may be
>> a benefit to have the results in a data structure that's easy to pass
>> along in one go, but if you don't want that, getting them out is a bit
>> of a hassle perhaps.
> I prefer this over (1), and I can see benefits of having a structure
> to pass around. If you take this route, I'd vote for a higher-level
> structure than a simple map -- Let users call isValid() to check for
> data integrity, and query the format that was used to generate this
> structure.
Yes, I agree, that might be the best route to go.
>
>
>> 3) QString::arg like API
>> An interesting option is to do something like this:
>>
>> int minutes, seconds;
>> if ( getUnits().unit(Qt::Minutes, minutes).unit(Qt::Seconds, seconds) ) {
>>     //blah
>> };
>>
>> Downside is that it allows specifying the same unit multiple times. That
>> does not need to be a real problem (simply return the same value more
>> than once), but it may look weird. Also, this API is implementation wise
>> quite a bit more complex than the previous options. The resulting code
>> is more verbose than option 1, but much easier to read.
> IMHO, (2) and (4) look "much easier to read" :)  (Having said that, I
> usually prefer concatenation to QString::args())
I usually don't, and actually like this chaining style where it makes 
sense. But I'm not saying that this is a good candidate for that :-)
> By the way, I'm not sure about calling the function "unit()". To me,
> it suggests returning a Qt::TimeSpanUnit rather than a number.
Absolutely. Naming is not fixed anywhere.
>> 4) separate requests
>> Another option is not return all relevant units in one go, but to use
>> multiple function calls:
>> int getUnit(Qt::TimeSpanFormat format, Qt::TimeSpanUnit unit);
>>
>> Failure would return -1.
> If you don't plan to support negative time spans, then returning -1
> would be a great way to indicate errors for different APIs.
Actually, negative spans do exist, so using negative return values to 
indicate failure is a no-go.
>> The example would yield:
>>
>> Qt::TimeSpanFormat format = Qt::Minutes | Qt::Seconds;
>> int minutes = getUnit(format, Qt::Minutes);
>> int seconds = getUnit(format, Qt::Seconds);
>> if (minutes >= 0 && seconds >=0) {
>>     //blah
>> }
>>
>> Downside is that in order to make this efficient, you'd need to cache
>> the result, otherwise you have to reconstruct it for every call with the
>> same format.
> See below.
>
>
>> Personally, I don't like having to make multiple function
>> calls to retrieve parts of the same answer. Also, there are
>> opportunities for errors here, such as asking for units that are not in
>> the format.
> (2) also requires "multiple calls" to extract the numeric values from
> the map. The only difference is that (2) makes multiple calls on the
> processed result, (4) makes multiple calls on the original data.
For me, that is a big difference actually...
> For
> (2), if timeSpan.getUnits(format).value(Qt::Months) returns 0, is it
> because the period is less than a month, or because I didn't ask for
> months in the format?
Well, in my original version of 2) using the Map, you could of course 
use ::contains(Qt::Month) for that. I think that when using a 
purpose-made data structure for this, it probably should support that 
kind of query API as well. I'm a bit afraid that this simple value 
container grows to have quite a big API itself if I'm not careful...
>
> In any case, I think the Qt-ish way IS to use different calls to
> get/set different things, instead of compressing everything into one
> line in the style of (1). See "The Convenience Trap" at
> http://qt-project.org/wiki/API-Design-Principles
I'm aware of that paper, but I don't think it (or at least this part of 
it) applies here. That part of the document is more about setter API's 
than about getting a set of parts of the same value out of a function 
call. The point is, that the units are not different things. They all 
represent exactly the same: an amount of time, but so in different 
magnitudes. It is not the case that a period of time really consists of 
days and hours and minutes, but it can be expressed that way. It can 
also be expressed using different units (representing different 
magnitudes). The combination of units (magnitudes) you wish to use has a 
direct impact on the returned values for each of these units. For 
instance, say that you don't want to use hours, then you might get a 
result of 236 minutes. If you would have used hours, then 59 would have 
been the maximum number of minutes that would have been returned.
>
>
>> I guess there are more options, but these four are the more serious
>> candidates I could come up with. Did I overlook a good candidate? Where
>> are other examples in the Qt API where there were similar needs, so I
>> can look if their patterns may apply here?
> For format-setting, QAudioInput's constructor takes a format to
> determine how it packages incoming data.
>
> For error reporting, many Qt functions return -1 to indicate an error.
> If you want to support negative spans, there's QString::toInt(bool
> *ok) which reports errors through an input pointer.
I guess that if the function is going to return a custom data type, that 
data type might as well include an isValid() method of its own to signal 
an error condition. As in your example below.
>
> In general, there are high-level convenience classes/functions for many things.
>
> What do you think of combining elements of these?
>
>      QTimeSpan timeSpan(...);
>      QTimeSpanDetails det = timeSpan.getDetails(Qt::Minutes|Qt::Seconds);
>
>      if (!det.isValid())
>          complain();
>
>      Qt::TimeSpanFormat fmt = det.format();
>
>      int mins = det.value(Qt::Minutes);
>      int secs1 = det.value(Qt::Seconds);
>
>      // Convenience functions
>      int secs2 = det.seconds();
>      int secs3 = det[Qt::Seconds];
>      int days = det.days();
>
>      if (mins == -1 || secs1 == -1 || secs2 == -1 || secs3 == -1)
>          complain();
>      if (days != -1)
>          complain();
>
> This takes care of the caching problem -- each representation is
> fixed-format after construction.
I think I'll go for something like this, though I'll start with a 
version without all the convenience functions. I was trying to trim the 
API I already had on QTimeSpan, and moving it to QTimeSpanDetails isn't 
much of a solution for that :-) They can always be added later if there 
is a need for it.
I am not sure about the name either; I'll think about that for a bit. 
It's the idea that counts though, and it seems like the idea is sound.

Thanks,

André