[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: how to enable telling KLocale::formatByteSize which format it
From:       David Faure <faure () kde ! org>
Date:       2009-03-02 19:23:53
Message-ID: 200903022023.54074.faure () kde ! org
[Download RAW message or body]

On Saturday 28 February 2009, Marcel Partap wrote:
> > Make a new method, and add an enum parameter to that method.
> > Then port the existing method to call that new method (but leave its
> > declaration in the header file unchanged).
> ok thx david actually that's what i did have in mind.. so now here's 
> the first patch version, works but still needs a few lines of 
> commenting.. 
... and unit test additions ;)

The name could be simplified indeed. Maybe formatByteSizeTo()?

Some comments about the API:

1) don't use a bool longPrefix, use an enum {LongFormat, ShortFormat}
 (why call it "prefix" when it's about the suffix, in English at least? ;)
 Maybe LongFormat is too generic (especially in KLocale which has date formats etc.),
  something like LongSizeFormat and ShortSizeFormat would be better,
  even if it's a bit longer to type.

2) the int precision needs documentation, but also maybe a default value?
I see that the existing code calls it always with 1, so maybe the default value
should be 1. If a good default can be found for the enum too, then the calling
code can be simplified in most cases to
  locale->formatByteSizeTo(size, KLocale::GigaByte)

-- 
David Faure, faure@kde.org, sponsored by Qt Software @ Nokia to work on KDE,
Konqueror (http://www.konqueror.org), and KOffice (http://www.koffice.org).
[prev in list] [next in list] [prev in thread] [next in thread]