[Development] std::chrono getters in our API

[Marc Mutz]

On Friday 10 June 2016 19:53:00 Thiago Macieira wrote:
> I've added a set of std::chrono API to QTimer[1] and QDeadlineTimer[2] and
> we're hitting a snag on what to name the getters. The setters are fine
> because they're just overloads:
> 
> 	timer.setInterval(3);		// Qt; milliseconds
> 	timer.setInterval(3ms);
> 	deadline.setRemainingTime(3600000); 	// milliseconds
> 	deadline.setRemainingTime(1h);
> 	deadline.setDeadline(QDeadlineTimer::current().deadline() + 3600000);
> 	deadline.setDeadline(std::chrono::steady_clock::now() + 1h);
> 
> The problem are the getters: what do we call them? We can't overload on
> return value, so we can't use the standard getter name matching the
> setters above, as it's already used for the Qt-style API:
> 
> 	int r = timer.remainingTime();		// milliseconds
> 
> I implemented an overload by way of templates:
> 
> 	auto r = timer.remainingTime<std::chrono::seconds>();
> 
> But some reviewers didn't like it and want the function to be non-template,
> returning std::chrono::milliseconds, leaving the conversion to a different
> type left as an exercise to the user.
> 
> So: what do we do?
> 
> Option 1:
> - Use overload-by-template like I did
> - Cons:
> 	requires a template argument
> - Pros:
> 	avoids ugly
> std::chrono::duration_cast<std::chrono::seconds>(timer.remainingTime());

The external duration_cast isn't really necessary, because all calculations 
will work properly as long as you use auto:

     auto remaining = timer.timeRemainingAsDuration(); // Option 2 naming
     auto nextTimerStart = remaining + 1h;

You only need to use duration_cast when you want to explicitly convert to a 
duration with less precision, usually only for display.
 
> Option 2:
> - Find a different name, not matching the setter name
> - Cons:
> 	doesn't match setter name
> - Pros:
> 	non-template

To fill this abstracly-described option with life: we're talking about 
something like:

     std::chrono::milliseconds timeRemainingAsDuration() const


> Option 3:
> - Find a different name for both setters and getters
> - Cons:
> 	can't write
> 		deadline.setRemainingTime(250ms);
> - Pros:
> 	clean API, but with some surprise factor
> 
> Option 4:
> - Find a different name for setters and getters, plus overload setters
> - Cons:
> 	a lot more template code in qtimer.h and qdeadlinetimer.h
> - Pros:
> 	clean API, but with some surprise factor
> 
> Option 5:
> - Drop std::chrono API
> - Cons:
> 	no QTimer::singleShot(5s, ...);
> - Pros:
> 	easiest
> 
> NON Option:
> - Use std::chrono only
> - Why:
> 	can't depend on it as the only way to access the time. Not to mention 
that
> 	QTimer already has ABI set.
> [1] https://codereview.qt-project.org/160889
> [2] https://codereview.qt-project.org/159932

-- 
Marc Mutz <marc.mutz at kdab.com> | Senior Software Engineer
KDAB (Deutschland) GmbH & Co.KG, a KDAB Group Company
Tel: +49-30-521325470
KDAB - Qt, C++ and OpenGL Experts