Re: Alternative to QDateTime::isDateOnly ?
Christian Mollekopf wrote: 4. Would be pretty good IMO, but unfortunately leads to an unexpressive interface (because QVariant can't be parametrized with valid values). You would just document in the API documentation what types the returned QVariant can take. Kevin Kofler
Re: Alternative to QDateTime::isDateOnly ?
On Tuesday 28 April 2015 22:32:12 Christian Mollekopf wrote: On the other hand, the problem of the variable return type is not solved by that, overloads and separate getters always seemed like a workaround to me, so it seems like Qt would benefit from something like boost::variant (aka. a discriminated union) Why wouldn't QVariant be enough? -- Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org Software Architect - Intel Open Source Technology Center PGP/GPG: 0x6EF45358; fingerprint: E067 918B B660 DBD1 105C 966C 33F5 F005 6EF4 5358
Re: Alternative to QDateTime::isDateOnly ?
On Tue, April 28, 2015 9:32 pm, Christian Mollekopf wrote: On Tue, Apr 28, 2015, at 08:47 PM, John Layt wrote: On 27 April 2015 at 21:17, Christian Mollekopf chrig...@fastmail.fm wrote: 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). We don't need a timezone for date-only (AFAIR), but we do need date-time sometimes. There *is* a valid reason for having a time zone for date-only values - it is then possible to determine whether a date-time value falls within that date-only value. For example, 2015-02-27T00:30 in an Australian time zone is during 2015-02-26 for a European time zone; it is earlier than 2015-02-27 in a European time zone. -- David Jarvie. KDE developer. KAlarm author - http://www.astrojar.org.uk/kalarm
Re: Alternative to QDateTime::isDateOnly ?
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. 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. Thanks for your input! Christian
Re: Alternative to QDateTime::isDateOnly ?
On Wed, Apr 29, 2015, at 12:33 AM, Aleix Pol wrote:
On Tue, Apr 28, 2015 at 10:11 PM, Christian Mollekopf
chrig...@fastmail.fm wrote:
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.
It avoids encoding the type into the name, but yes, it's essentially the
same.
- variantSomething/QPair
It's hard to read the code afterwards.
I don't find a variantT1, T2 hard to read.
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;
}
Definitely a good suggestion in line with John's Duration.
What I like about the discriminated union [0] approach is that it
solves pretty much the same problem,
using a more generic solution.
The only good argument I'd see for an Occasion class would be if it
offered additional helpers that are useful
to both date-only and date-time occasions. I.e. a date() accessor, that
returns the date of either date-time or date.
Sooo I'm still not entirely sure, but something similar like
boost::variant (a discriminated union), doesn't seem like the worst
idea, but something like Occasion is not that far off either. I'll sleep
over it again ;-)
Cheers,
Christian
[0] http://www.drdobbs.com/cpp/discriminated-unions-i/184403821?pgno=2
Re: Alternative to QDateTime::isDateOnly ?
On Tue, Apr 28, 2015, at 08:47 PM, John Layt wrote: On 27 April 2015 at 21:17, Christian Mollekopf chrig...@fastmail.fm wrote: Hey John, 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. Ok. 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). We don't need a timezone for date-only (AFAIR), but we do need date-time sometimes. 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? That's seems like too much of a hack for me. 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. Not a bad idea at all... The end/due date must follow the start date by definition after all. On the other hand, the problem of the variable return type is not solved by that, overloads and separate getters always seemed like a workaround to me, so it seems like Qt would benefit from something like boost::variant (aka. a discriminated union) 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... True, a separate Duration class that can either point to a day (date-only), or a second (date-time) could be a solution as well. Thanks for your ideas, Christian
Re: Alternative to QDateTime::isDateOnly ?
Hey,
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?
I also think that Aleixs approch is not good, because you can't tell if the
QDateTime is invaliad at all or a dateonly value. Create a childclass
QDateTime with a dateOnly attribute and change isValid. If this method is
virtual, we can also think about designing the API only with the baseclass
QDateTime and not with the childclass.
Personally, I suspect relying on the QDateTime to tell you it is Date
Only is perhaps the wrong approach?
The problem is not how we save the date/datetime internally, therefore we
could use Aleix aproch (the event has actually the attribute dateonly). The
problem is interface to the outside world. It would be nice to have an as easy
interface than possible and for that it would be nice to have only one
setter/getter for ex. the start attribute. Because if we have no type that
handles both you have evey time to check expictly and copy code:
if (event.isDateOnly()) {
QDate date = event.startDate();
[...]
} else {
QDateTime datetime = event.startDateTime();
[...]
}
against:
DATETYPE dateOrDateTime = event.start();
[...]
if (dateOrDateTime.isDateOnly()) {
[...]
}
But look at usecases, why we need to know if it is a dateOnly datepoint:
* dateonly is shown at over views in calendar (without timezoneinfo it is
translated to local time)
* enable/disable widgets for time/timezone
* rfc has the difference, so we need to know when we save/load from ical
f.ex. normally nobody sets bithdays with time, but rfc allows it.
* for a valid time you need for sure a valid timezone otherwise a time doesn't
make sense.
Regads,
sandro
--
Sandro Knauß
Software Developer
Kolab Systems AG
Zürich, Switzerland
e: kna...@kolabsystems.com
t: +41 43 501 66 91
w: https://kolabsystems.com
pgp: CE81539E Sandro Knauß
signature.asc
Description: This is a digitally signed message part.
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
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.
