--===============3665928207683434104== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable > On 2010-10-26 08:21:12, Sebastian Trueg wrote: > > /trunk/KDE/kdelibs/kdecore/date/kdate.h, line 607 > > > > > > an example would be good here since it is rather unclear what the m= ethod (and its friends below) return. > = > John Layt wrote: > I've reworked the apidox. I'm now rethinking the methods entirely. = The KCalendarSystem api for yearString() and the rest is needed because you= can't just display the year() number in the gui, the localized string form= may have a different digit set or even a non-decimal or non-numeric form (= Hebrew is the big case here). However, the more recent addition of a forma= tDate() version to the KCalendarSystem api where you can pass in your own f= ormat string (e.g. "%Y" for the long year) makes the reason for having them= moot. I could remove them entirely, but the whole purpose of KLocalizedDa= te is to provide localized versions in a simple way. Perhaps slightly modi= fy the name to remove the "String" and remove the default value to look lik= e: > = > int year() const; > QString year(KCalendarSystem::StringFormat format) const; > = > I think that might make it more obvious? > = > Sebastian Trueg wrote: > why not formatYear() to match the big brother method? Nice, I'll take that. I was a little uneasy about using the same method na= me with different return types. Also allows me to set default values too. > On 2010-10-26 08:21:12, Sebastian Trueg wrote: > > /trunk/KDE/kdelibs/kdecore/date/kdate.h, line 736 > > > > > > Confusing that this is not a static method. > = > John Layt wrote: > I had initially made it static, but I wondered if there would then be= confusion as to what Calendar System would be used if accessed from an ins= tantiated KDate rather than statically, e.g. called as myDate.readDate() ra= ther than KDate::readDate(). Would the user expect the instantiated date's= calendar system to be used rather than the global that it really would? O= r should having clear documentation be enough? > = > Sebastian Trueg wrote: > IMHO a static version with an optional KCaldendarSystem parameter mig= ht do the trick. Although the latter might even be superfluous since using = a custom caldendarsystem is not a simple usage pattern anymore in which cas= e it would be ok for the developer to go back to using KCalendarSystem. Yep, I'll probably go with the basic static and a big flashing neon warning= :-) The other problem with passing in a calendar system to use is you are= likely to be wanting a different locale with it too, i.e. different langua= ge or date format, so that's better using KCS anyway. - John ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://svn.reviewboard.kde.org/r/5692/#review8358 ----------------------------------------------------------- On 2010-10-25 20:54:08, John Layt wrote: > = > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://svn.reviewboard.kde.org/r/5692/ > ----------------------------------------------------------- > = > (Updated 2010-10-25 20:54:08) > = > = > Review request for kdelibs. > = > = > Summary > ------- > = > The KCalendarSystem api for localizing dates is awkward, inconvenient, un= intuitive, and long-winded, causing many mistakes to be made or localizatio= n to be ignored altogether. This change adds a new KDate class designed to= make localizing dates as easy as using QDate. > = > Some QDate code may look like: > = > QDate myDate( aYear, aMonth, aDay ); > int doy =3D myDate.dayOfYear(); > = > The KDE localized date code looks something like: > = > QDate myDate; > KGlobal::locale()->calendar()->setDate( myDate, aYear, aMonth, aDay ); > int doy =3D KGlobal::locale()->calendar()->dayOfYear( myDate ); > = > The localized KDate code would look like: > = > KDate myDate( aYear, aMonth, aDay ); > int doy =3D myDate.dayOfYear(); > = > Much easier. > = > KDate is a thin wrapper around KCalendarSystem and QDate, with a near ide= ntical api to QDate and as such can be used as a drop-in replacement with v= ery few changes. Some deprecated or unnecessary KCalendarSystem methods ha= ve not been included, but these can still be accessed via the calendar() me= thods. Some new convenience methods have also been added such as setCurren= tDate() and addYearsOn(). > = > Some methods have QDate overloads for convenience, and the assignment and= comparison operators partially work with QDate on the rhs. If anyone know= s how to make it work with QDate on the lhs, or any other QDate compatibili= ty ideas, I'm all ears. > = > For now I only intend this to be used as a convenience class by apps inte= rnally and not to be used in kdelibs api as I don't see much advantage in t= hat, but I may do so if the demand for convenience is there. > = > I have named it KDate, but there is the possibility people may get confus= ed and think that KDateTime also localizes datetime's, but that is not the = case. If people think this will be a problem KLocalizedDate is an alternat= ive if more awkward name. > = > = > Diffs > ----- > = > /trunk/KDE/kdelibs/kdecore/CMakeLists.txt 1189756 = > /trunk/KDE/kdelibs/kdecore/date/kdate.h PRE-CREATION = > /trunk/KDE/kdelibs/kdecore/date/kdate.cpp PRE-CREATION = > /trunk/KDE/kdelibs/kdecore/tests/kcalendartest.h 1189756 = > /trunk/KDE/kdelibs/kdecore/tests/kcalendartest.cpp 1189756 = > = > Diff: http://svn.reviewboard.kde.org/r/5692/diff > = > = > Testing > ------- > = > Full unit tests included. > = > = > Thanks, > = > John > = > --===============3665928207683434104== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable
This is an automatically generated e-mail. To reply, visit: http://svn.reviewb= oard.kde.org/r/5692/

On October 26th, 2010, 8:21 a.m., Sebastian= Trueg wrote:

= = =
/trunk/KDE/kdelibs/kdecore/date/kdate.h (Diff revision 1) 

   
public:


  

 

  

    
    

    607
    
    QString dayOfYearString(=
KCalendarSystem::StringFo=
rmat format =3D KCalendarSystem::LongFormat) const=
;

  


 




  
an exampl=
e would be good here since it is rather unclear what the method (and its fr=
iends below) return.

 




 
On October 26th, 2010, 9:48 p.m., John Layt wrote:

 

  
I've =
reworked the apidox.  I'm now rethinking the methods entirely.  The KCa=
lendarSystem api for yearString() and the rest is needed because you can=
9;t just display the year() number in the gui, the localized string form ma=
y have a different digit set or even a non-decimal or non-numeric form (Heb=
rew is the big case here).  However, the more recent addition of a formatDa=
te() version to the KCalendarSystem api where you can pass in your own form=
at string (e.g. "%Y" for the long year) makes the reason for havi=
ng them moot.  I could remove them entirely, but the whole purpose of KLoca=
lizedDate is to provide localized versions in a simple way.  Perhaps slight=
ly modify the name to remove the "String" and remove the default =
value to look like:

    int year() const;
    QString year(KCalendarSystem::StringFormat format) const;

I think that might make it more obvious?

 






 
On October 29th, 2010, 3 p.m., Sebastian Trueg wrote:

 

  
why not f=
ormatYear() to match the big brother method?

 











Nice, I'll take that.  I was a little uneasy about using the sam=
e method name with different return types.  Also allows me to set default v=
alues too.






 
On October 26th, 2010, 8:21 a.m., Sebastian=
 Trueg wrote:

 

  =









 =

 =




 

 
  

   
    /trunk/KDE/kdelibs/kdecore/date/kdate.h
    

     (Diff revision 1)

    
   
  

 

 
  


   

   
public:


  

 

  

    
    

    736
    
    KDate readDate(const QString &dateString, boo=
l *ok =3D 0) const;

  


 




  
Confusing=
 that this is not a static method.

 




 
On October 26th, 2010, 10:36 p.m., John Layt wrote:

 

  
I had ini=
tially made it static, but I wondered if there would then be confusion as t=
o what Calendar System would be used if accessed from an instantiated KDate=
 rather than statically, e.g. called as myDate.readDate() rather than KDate=
::readDate().  Would the user expect the instantiated date's calendar s=
ystem to be used rather than the global that it really would?  Or should ha=
ving clear documentation be enough?

 






 
On October 29th, 2010, 3:02 p.m., Sebastian Trueg wrote:

 

  
IMHO a st=
atic version with an optional KCaldendarSystem parameter might do the trick=
. Although the latter might even be superfluous since using a custom calden=
darsystem is not a simple usage pattern anymore in which case it would be o=
k for the developer to go back to using KCalendarSystem.

 











Yep, I'll probably go with the basic static and a big flashing n=
eon warning :-)  The other problem with passing in a calendar system to use=
 is you are likely to be wanting a different locale with it too, i.e. diffe=
rent language or date format, so that's better using KCS anyway.








- John






On October 25th, 2010, 8:54 p.m., John Layt wrote:









 

  


Review request for kdelibs.


By John Layt.




Updated 2010-10-25 20:54:08






Descripti=
on 



 

  
   
The KCalendarSystem api for localizing dates is awkward, inc=
onvenient, unintuitive, and long-winded, causing many mistakes to be made o=
r localization to be ignored altogether.  This change adds a new KDate clas=
s designed to make localizing dates as easy as using QDate.

Some QDate code may look like:

    QDate myDate( aYear, aMonth, aDay );
    int doy =3D myDate.dayOfYear();

The KDE localized date code looks something like:

    QDate myDate;
    KGlobal::locale()->calendar()->setDate( myDate, aYear, aMonth, aD=
ay );
    int doy =3D KGlobal::locale()->calendar()->dayOfYear( myDate );

The localized KDate code would look like:

    KDate myDate( aYear, aMonth, aDay );
    int doy =3D myDate.dayOfYear();

Much easier.

KDate is a thin wrapper around KCalendarSystem and QDate, with a near ident=
ical api to QDate and as such can be used as a drop-in replacement with ver=
y few changes.  Some deprecated or unnecessary KCalendarSystem methods have=
 not been included, but these can still be accessed via the calendar() meth=
ods.  Some new convenience methods have also been added such as setCurrentD=
ate() and addYearsOn().

Some methods have QDate overloads for convenience, and the assignment and c=
omparison operators partially work with QDate on the rhs.  If anyone knows =
how to make it work with QDate on the lhs, or any other QDate compatibility=
 ideas, I'm all ears.

For now I only intend this to be used as a convenience class by apps intern=
ally and not to be used in kdelibs api as I don't see much advantage in=
 that, but I may do so if the demand for convenience is there.

I have named it KDate, but there is the possibility people may get confused=
 and think that KDateTime also localizes datetime's, but that is not th=
e case.  If people think this will be a problem KLocalizedDate is an altern=
ative if more awkward name.

  		
 






Testing <=
/h1>


 

  
   
Full unit tests included.

  		
 








Diffs=
 


    

 
  • /trunk/KDE/kdelibs/kdecore/CMakeLists.txt =
(1189756)
    • 

 
  • /trunk/KDE/kdelibs/kdecore/date/kdate.h (P=
RE-CREATION)
    • 

 
  • /trunk/KDE/kdelibs/kdecore/date/kdate.cpp =
(PRE-CREATION)
    • 

 
  • /trunk/KDE/kdelibs/kdecore/tests/kcalendartest.h (1189756)
    • 

 
  • /trunk/KDE/kdelibs/kdecore/tests/kcalendartest.cpp (1189756)
    • 





View Diff





  		
 











  

 



--===============3665928207683434104==--