Skip to content

Have ordinal return 0th instead of 0 #80

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.

Sign up for GitHub

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

Open
DZittersteyn wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Have ordinal return 0th instead of 0 #80

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1ce7cd6
Have ordinal return 0th instead of 0
Jul 6, 2016
223fcd2
add zeroth option to ordinal
Jul 7, 2016
49be394
add spec for shortcutting behavior
Jul 7, 2016
fd4b699
Add zeroth option to readme
Jul 7, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,17 @@ Humanize.ordinal(22)
// "22nd"
```

By default returns the input if it evaluates to 0, this can be overridden by passing the option `zeroth`:

```javascript
Humanize.ordinal(0)
// 0
Humanize.ordinal('0.')
// "0."
Humanize.ordinal(0, {zeroth: true})
// "0th"
```

##### times
Interprets numbers as occurences. Also accepts an optional array/map of overrides.

Expand Down
10 changes: 9 additions & 1 deletion __tests__/humanize.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,18 @@ describe('compactInteger tests', () => {

describe('Ordinal value of numbers Test Suite', () => {
describe('Ordinal value for numbers ending in zero', () => {
it('should return 0 if the number is 0 (cos 0th doesnt read very well)', () => {
it('should return 0 if the number is 0', () => {
expect(Humanize.ordinal(0)).toEqual(0);
});

it('should return the input if the number is 0', () => {
expect(Humanize.ordinal('0.')).toEqual('0.');
Copy link
Contributor

@geekjuice geekjuice Jul 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if the method should be updated to return the string version of the parsed input rather than the original input.

expect(Humanize.ordinal(0)).toEqual('0');

// and 

expect(Humanize.ordinal('0.foobar')).toEqual('0');

Copy link
Author

@DZittersteyn DZittersteyn Jul 8, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would make sense to me, since ordinal(1.foo) returns 1st, not 1.foost. It would however be somewhat of an backwards incompatible change, so I'll shoot in a separate PR for that

});

it('should return 0th if the number is 0, and the zeroth option is passed', () => {
expect(Humanize.ordinal(0, {zeroth: true})).toEqual('0th');
});

it('should return the number with suffix th', () => {
expect(Humanize.ordinal(10)).toEqual('10th');
});
Expand Down
5 changes: 3 additions & 2 deletions src/humanize.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,10 +217,11 @@
},

// Converts an integer to its ordinal as a string.
ordinal(value) {
ordinal(value, options = {}) {
const { zeroth = false } = options;
const number = parseInt(value, 10);

if (number === 0) {
if (number === 0 && !zeroth) {
return value;
}

Expand Down