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

Already on GitHub? Sign in to your account

2.3 CakeNumber::formatDelta() #771

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
4 participants
Member

dereuromark commented Aug 13, 2012

in some cases you just want to display the values as signed values in the view than first figuring out the sign and passing this on to the format() method.
this is especially useful for outputting differences in values (e.g. "+50 meters since xxxx-xx-xx").

implementing it I found a related issue with format() and almost-zero values. fixed and test cases added.
-0.01 would not show as -0.0 anymore for format() now anymore.

note: used to be #658

Owner

lorenzo commented Aug 13, 2012

I like the idea, but not the method name that much

Member

dereuromark commented Aug 13, 2012

@lorenzo you had like months to complain :) hm, any ideas for it?

Owner

lorenzo commented Aug 13, 2012

Dd not follow the other pull request, sorry :(

Member

dereuromark commented Aug 13, 2012

no problem. what about formatDelta()?

Member

dereuromark commented Sep 6, 2012

@lorenzo How do you like CakeNumber::formatDelta()? After all, that's what it is. I can modify the PR then.

Owner

lorenzo commented Sep 19, 2012

Votes for/against this PR?

Member

dereuromark commented Sep 19, 2012

don't forget the bugfix about almost-zero values and the sign being displayed otherwise (-0)

@markstory markstory commented on an outdated diff Sep 19, 2012

lib/Cake/Utility/CakeNumber.php
@@ -158,6 +159,27 @@ public static function format($number, $options = false) {
}
/**
+ * Formats a number into a currency format to show deltas (as signed value).
+ *
+ * @param float $number A floating point number
+ * @param integer $options if int then places, if string then before, if (,.-) then use it
+ * or array with places and before keys
+ * @return string formatted difference
+ */
+ public static function formatDelta($value, $options = array()) {
+ $places = isset($options['places']) ? $options['places'] : 0;
+ $value = self::_numberFormat($value, $places, '.', '');
+ $sign = $value > 0 ? '+' : '';
+ $options = (array)$options;
@markstory

markstory Sep 19, 2012

Owner

Shouldn't this happen first? I remember in that in 5.2.x `isset($var['key']) on a string is always true. I don't see why we need to accept mixed types here.

@markstory markstory and 1 other commented on an outdated diff Sep 19, 2012

lib/Cake/Utility/CakeNumber.php
@@ -158,6 +159,27 @@ public static function format($number, $options = false) {
}
/**
+ * Formats a number into a currency format to show deltas (as signed value).
+ *
+ * @param float $number A floating point number
+ * @param integer $options if int then places, if string then before, if (,.-) then use it
@markstory

markstory Sep 19, 2012

Owner

Isn't this parameter an array?

@dereuromark

dereuromark Sep 19, 2012

Member

well, not that easy, i am afraid - it uses format() - and its doc: "integer $options if int then places, if string then before, if (,.-) then use it or array with places and before keys"
so it can be multiple things at once (which I dont like by the way)

@markstory

markstory Sep 19, 2012

Owner

Ugh, its probably still a good idea to fix the isset() and do type checks on the parameter instead.

@markstory markstory and 1 other commented on an outdated diff Sep 19, 2012

lib/Cake/Utility/CakeNumber.php
+ *
+ * @param float $number A floating point number
+ * @param integer $options if int then places, if string then before, if (,.-) then use it
+ * or array with places and before keys
+ * @return string formatted difference
+ */
+ public static function formatDelta($value, $options = array()) {
+ $places = isset($options['places']) ? $options['places'] : 0;
+ $value = self::_numberFormat($value, $places, '.', '');
+ $sign = $value > 0 ? '+' : '';
+ $options = (array)$options;
+ if (isset($options['before'])) {
+ $options['before'] .= $sign;
+ } else {
+ $options['before'] = $sign;
+ }
@markstory

markstory Sep 19, 2012

Owner

Couldn't this whole if/else be written as $options['before'] = isset($options['before']) ? $options['before'] . $sign : $sign; ?

@dereuromark

dereuromark Sep 19, 2012

Member

sure, I just copy and pasted the old format method here. But I can refactor accordingly

@jrbasso jrbasso and 1 other commented on an outdated diff Sep 19, 2012

lib/Cake/Utility/CakeNumber.php
@@ -158,6 +159,27 @@ public static function format($number, $options = false) {
}
/**
+ * Formats a number into a currency format to show deltas (as signed value).
+ *
+ * @param float $number A floating point number
@jrbasso

jrbasso Sep 19, 2012

Member

This param is called $value, not $number.

@dereuromark

dereuromark Sep 19, 2012

Member

I correct this and rename all $number into $value

Member

dereuromark commented Sep 19, 2012

maybe we should only accept an array here. providing the different string/int params as well makes it way more complicated without gaining anything. it is a new method anyway (so no BC issues.

Owner

markstory commented Sep 19, 2012

That's probably a better option :)

Member

dereuromark commented Sep 19, 2012

closing in favor of #859 which is not screwed up by some "fast forward" mistakes i made.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment