Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

I18n\Formatter vs. logic #3

Closed
etki opened this issue Nov 30, 2014 · 6 comments
Closed

I18n\Formatter vs. logic #3

etki opened this issue Nov 30, 2014 · 6 comments

Comments

@etki
Copy link

etki commented Nov 30, 2014

Hi.

I've stumbled upon this piece of code

// sizeFormatBase is used here to determine the actual scale of value
list($params, $position) = $this->formatSizeNumber($value, $decimals, $options, $textOptions);
if ($this->sizeFormatBase == 1024) {
    // format as kibibytes
} else {
    // format as kilobytes
}

I see this as an erm something that should have never hit production. It proposes following things:

  1. Set some internal property to get different formatting. Well, if i want another behavior (not state), i should call another method or the same method with other parameters, shouldn't i?
  2. Whenever i'm counting kilobytes (and i mean jargon 1024-based meaning), i'm not counting kilobytes. I'm counting something close to kilobytes. Or not. May be my colleague will set $sizeFormatBase to 2 or -0.15. That would be tremendous joke, i'd say.
  3. If you want to format anything as kibibytes after you've formatted something other as kilobytes, just don't forget to switch $sizeFormatBase back to... I guess, you get the problem, state vs. behavior problem again. And a grand canyon-wide field for bugs and debugging that could be easily avoided.
  4. There's even no space for $formatter->sizeFormatBase = 1024.0; hack. I thought nobody was using soft comparison nowadays.
@cebe
Copy link
Member

cebe commented Nov 30, 2014

Except from the fact that this could be implemented in a better way the documentation about sizeFormatBase states the available configuration options are 1000 or 1024. If you input something else its okay if it does not work. https://github.com/yiisoft/yii2/blob/c6b0e24a456d19092b47d9296fdd739b8899cded/framework/i18n/Formatter.php#L195

To allow formatting with different bases in the same application we could add a parameter to make it explicit per format which base to use.

@etki
Copy link
Author

etki commented Nov 30, 2014

@cebe

/**
 * @var array ... Defaults to 1024.
 */

That's just got even more weird.

the documentation about sizeFormatBase states the available configuration options are 1000 or 1024

Yes! But only in case you've been smart enough to walk all the way down to that property.

 * Formats the value in bytes as a size in human readable form for example `12 KB`.
 *
 * This is the short form of [[asSize]].
 *
 * If [[sizeFormatBase]] is 1024, [binary prefixes](http://en.wikipedia.org/wiki/Binary_prefix) (e.g. kibibyte/KiB, mebibyte/MiB, ...)
 * are used in the formatting result.

(what does mean if it is 1024? if i want something else, would be false okay?)

And, erm, i wouldn't even look into docs for method asSize(). I'd look into parameters.

If you input something else its okay if it does not work.

I didn't even test it, but i guess that it works and outputs nearly anything as kilobytes, megabytes, etc.

@samdark
Copy link
Member

samdark commented Jan 15, 2017

I don't think it's possible to change it in 2.0.x but in 2.1.x we may adjust it to be two separate methods: one for kilobytes and one for kibibytes.

@samdark samdark transferred this issue from yiisoft/yii2 Nov 3, 2018
@samdark samdark transferred this issue from yiisoft/yii-core Apr 18, 2019
@mrblc
Copy link

mrblc commented May 20, 2019

I wouldn't call it state nor behavior, but configuration. UI must be consistent, so in any app with good UX, sizeFormatBase should be configured at application level and consistently used. Different methods could be useful, but ability to switch from one format to another in one place (e.g. config/main.php) is also important.

@samdark could you consider adding two new methods, but leaving current asSize() as proxy method to one of two new methods, depending of configuration?

P.S. It could be implemented in better way, for example boolean (e.g. useSIforSize).

@samdark
Copy link
Member

samdark commented May 20, 2019

@mrblc yes. Having two methods is better.

@samdark
Copy link
Member

samdark commented Nov 5, 2021

No longer applies to the code in this repository.

@samdark samdark closed this as completed Nov 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants