MINOR: adding SpelledOut method in DBInt#11096
Conversation
GuySartorelli
left a comment
GuySartorelli
left a comment
There was a problem hiding this comment.
Hi,
Thank you for this contribution. It seems like a good idea to me - just a few things to tidy up
| } | ||
|
|
||
|
|
||
| public function SpelledOut() |
There was a problem hiding this comment.
Please add a phpdoc comment explaining what the method does.
| } | ||
|
|
||
|
|
||
| public function SpelledOut() |
There was a problem hiding this comment.
| public function SpelledOut() | |
| public function SpelledOut(): string |
People will expect a string, but NumberFormatter::format() can return false.
This method should return an empty string in the event the format() method returns false.
| $v = $this->prepValueForDB($this->value); | ||
| return (new NumberFormatter(i18n::get_locale(), NumberFormatter::SPELLOUT))->format($v); |
There was a problem hiding this comment.
| $v = $this->prepValueForDB($this->value); | |
| return (new NumberFormatter(i18n::get_locale(), NumberFormatter::SPELLOUT))->format($v); | |
| $value = $this->prepValueForDB($this->value); | |
| $formatter = new NumberFormatter(i18n::get_locale(), NumberFormatter::SPELLOUT); | |
| return $formatter->format($value); |
Please don't use single-letter variables, nor wrap instantiation in quotes.
|
|
||
| public function SpelledOut() | ||
| { | ||
| $v = $this->prepValueForDB($this->value); |
There was a problem hiding this comment.
Why are you calling prepValueForDB() first? e.g. DBInt::Formatted() doesn't do that.
|
Please also create an issue for this PR and link them to each other as per our contribution guide |
2 participants
This is just an idea, be curious to see if this would be acceptable.
You can then write in your template:
$MyInt = $MyInt.SpelledOutWhich outputs: