Re: Alternative to QDateTime::isDateOnly ?
On Tue, Apr 28, 2015 at 8:47 PM, John Layt jl...@kde.org wrote: On 27 April 2015 at 21:17, Christian Mollekopf chrig...@fastmail.fm wrote: 1. add isDateOnly functionality to QDateTime ... Opinions following: 1. I'm not sure whether it semantically makes sense to have a QDateTime without a time. Adding it to QDateTime was not an option Thiago was happy with so it didn't make it, It is something only used inside PIM so there's no wide use-case for it. I do think I could add it fairly easily as a new internal attribute flag, but it could complicate the code a lot and I'm also not sure it's possible to do with a backwards compatible behaviour either. Using a QDate in the api is probably not an option for PIM though as it doesn't have a QTimeZone attached which you will certainly always need (and yes, that is a reason for doing it in QDateTime). One option is the invalid QTime that Aleix mentions. I did have that in the back of my mind while re-writing QDateTime internals and so whatever QDate, Qtime and QTimeZone you set should persist in spite of the QDateTIme overall being invalid. However it's not really a solution as how would you know when it was really invalid or when it was Date Only? Well, it would only be invalid if the date is also invalid then. It would require to change the semantics of ::isValid though, which is probably not accepted at this point anymore. Personally, I suspect relying on the QDateTime to tell you it is Date Only is perhaps the wrong approach? Perhaps it's actually an attribute of the Event that it is Date Only and not an attribute of the QDateTime? Rather than checking the QDateTime you've received from a call to start() to see if it is Date Only, you should be checking if the Event itself is Date Only? Your setter could have an enum for choosing, or separate setters for QDate and QDateTime, and then have a getter for QDateTIme and another for the enum. While this may seem like a little more work for PIM, it's not used in many places so I think this may be a better option, and easier to achieve too in the required timeframe as it's all in your own control. It would make sense to have a PointInTime with higher or lower accuracy though. I had pondered for a while having that, but with QDateTime internals now entirely held as milliseconds relative to the Unix epoch and translated into the QDate and QTime relative to the QTimeZone on the fly, that's effectively what QDateTime has become. Now, a QDuration class, or duration mode for QDateTime, could be useful though... Cheers! John.
Re: Alternative to QDateTime::isDateOnly ?
On Tue, Apr 28, 2015 at 10:11 PM, Christian Mollekopf
chrig...@fastmail.fm wrote:
Hey Aleix,
What about considering the port to be like:
QDateTime().time().isNull()?
Even QDateTime::isValid documentation mentions that the date and the
time need to be valid, therefore the time can be invalid.
With that assumption, I'd say we could even implement
QDateTime::isDateOnly() or similar.
I appreciate the pragmatism of that approach, but I just consider an
interface
that returns an invalid QDateTime fundamentally broken (tm).
I mean, that would be like the first thing I'd check in a unittest, and
the behaviour would IMO be completely unexpected.
Rock and roll! \\nn//
Let's not do it then. :)
I may be a bit extreme that way, but QDateTime::isValid() would be a
blocker
for the isDateOnly() functionality IMO.
I would most certainly not go into template stuff (i.e. 3, 4 and 5) 2
looks ok but if we get to add the API in Qt, we'll get to port things
much more easily.
I agree that the Qt solution would be the easiest, but why would you not
use the template solutions?
They actually seem to be the cleanest to me.
- valueT
Using templates when you know the 2 types it will have is not better
than just making both methods.
- variantSomething/QPair
It's hard to read the code afterwards.
What about having a new class (In KCoreAddons? KCalCore?) to replace
KDateTime in PIM?
Something like:
class Occasion
{
QDateTime dateTime() const;
QDate date() const;
bool isAllDay() const;
}
HTH,
Aleix
Alternative to QDateTime::isDateOnly ?
Hey, KDateTime used to have a date-only functionality, that QDateTime is lacking. The problem with that is that we need to find a new solution for interfaces that allow to set/get either QDate or QDateTime. One such interface is KCalCore::Event::start(). For the sake of discussion getters are more interesting, because a simple overload is not possible. I see the following possible solutions: 1. add isDateOnly functionality to QDateTime 2. Overloads with different names: QDate startDate(), QDateTime startDateTime() 3. Overloads using templates: T startT() 4. QVariant that can contain either QDateTime or QDate: QVariant start() 5. boost::variant that can contain either QDateTime or QDate: boost::variantQDate, QDateTime start() Given that this should be a fairly common porting problem (at least in the PIM realm), it would be nice to have a standard solution for it. Opinions following: 1. I'm not sure whether it semantically makes sense to have a QDateTime without a time. It would make sense to have a PointInTime with higher or lower accuracy though. 2. Not a solution, but a workaround. 3. Better than 2., but not by much. 4. Would be pretty good IMO, but unfortunately leads to an unexpressive interface (because QVariant can't be parametrized with valid values). 5. boost::variant solves the problem of 4., and is header-only, but I'm not sure to what extent boost is accepted in interfaces? I think the variant solutions are actually not that bad, semantically, but QVariant seems a bit useless for a case like this. Any ideas or opinions? Cheers, Christian
Re: Alternative to QDateTime::isDateOnly ?
On Mon, Apr 27, 2015 at 10:17 PM, Christian Mollekopf chrig...@fastmail.fm wrote: Hey, KDateTime used to have a date-only functionality, that QDateTime is lacking. The problem with that is that we need to find a new solution for interfaces that allow to set/get either QDate or QDateTime. One such interface is KCalCore::Event::start(). For the sake of discussion getters are more interesting, because a simple overload is not possible. I see the following possible solutions: 1. add isDateOnly functionality to QDateTime 2. Overloads with different names: QDate startDate(), QDateTime startDateTime() 3. Overloads using templates: T startT() 4. QVariant that can contain either QDateTime or QDate: QVariant start() 5. boost::variant that can contain either QDateTime or QDate: boost::variantQDate, QDateTime start() Given that this should be a fairly common porting problem (at least in the PIM realm), it would be nice to have a standard solution for it. Opinions following: 1. I'm not sure whether it semantically makes sense to have a QDateTime without a time. It would make sense to have a PointInTime with higher or lower accuracy though. 2. Not a solution, but a workaround. 3. Better than 2., but not by much. 4. Would be pretty good IMO, but unfortunately leads to an unexpressive interface (because QVariant can't be parametrized with valid values). 5. boost::variant solves the problem of 4., and is header-only, but I'm not sure to what extent boost is accepted in interfaces? I think the variant solutions are actually not that bad, semantically, but QVariant seems a bit useless for a case like this. Any ideas or opinions? Cheers, Christian What about considering the port to be like: QDateTime().time().isNull()? Even QDateTime::isValid documentation mentions that the date and the time need to be valid, therefore the time can be invalid. With that assumption, I'd say we could even implement QDateTime::isDateOnly() or similar. I would most certainly not go into template stuff (i.e. 3, 4 and 5) 2 looks ok but if we get to add the API in Qt, we'll get to port things much more easily. Aleix
Re: Alternative to QDateTime::isDateOnly ?
On 27 April 2015 at 21:17, Christian Mollekopf chrig...@fastmail.fm wrote: 1. add isDateOnly functionality to QDateTime ... Opinions following: 1. I'm not sure whether it semantically makes sense to have a QDateTime without a time. Adding it to QDateTime was not an option Thiago was happy with so it didn't make it, It is something only used inside PIM so there's no wide use-case for it. I do think I could add it fairly easily as a new internal attribute flag, but it could complicate the code a lot and I'm also not sure it's possible to do with a backwards compatible behaviour either. Using a QDate in the api is probably not an option for PIM though as it doesn't have a QTimeZone attached which you will certainly always need (and yes, that is a reason for doing it in QDateTime). One option is the invalid QTime that Aleix mentions. I did have that in the back of my mind while re-writing QDateTime internals and so whatever QDate, Qtime and QTimeZone you set should persist in spite of the QDateTIme overall being invalid. However it's not really a solution as how would you know when it was really invalid or when it was Date Only? Personally, I suspect relying on the QDateTime to tell you it is Date Only is perhaps the wrong approach? Perhaps it's actually an attribute of the Event that it is Date Only and not an attribute of the QDateTime? Rather than checking the QDateTime you've received from a call to start() to see if it is Date Only, you should be checking if the Event itself is Date Only? Your setter could have an enum for choosing, or separate setters for QDate and QDateTime, and then have a getter for QDateTIme and another for the enum. While this may seem like a little more work for PIM, it's not used in many places so I think this may be a better option, and easier to achieve too in the required timeframe as it's all in your own control. It would make sense to have a PointInTime with higher or lower accuracy though. I had pondered for a while having that, but with QDateTime internals now entirely held as milliseconds relative to the Unix epoch and translated into the QDate and QTime relative to the QTimeZone on the fly, that's effectively what QDateTime has become. Now, a QDuration class, or duration mode for QDateTime, could be useful though... Cheers! John.
