HiveBrain v1.2.0
Get Started
← Back to all entries
snippetMinor

Convert numbers into a textual form

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
convertintonumberstextualform

Problem

So this was originally inspired by @nhgrif's Int extension for translating integer to plain English, however the code is mainly a port of @rolfl's Converting a number to the text representation; taking into consideration the reviews on both answers as well.

NumberToText.h:

#import 

@interface NSString (NumberToText)

- (instancetype)initWithInteger:(NSInteger)number;
- (instancetype)initWithInteger:(NSInteger)number negativeSign:(NSString *)negativeSign;
+ (instancetype)stringWithInteger:(NSInteger)number;
+ (instancetype)stringWithInteger:(NSInteger)number negativeSign:(NSString *)negativeSign;
+ (NSString *)tripleAsText:(NSInteger)value;

@end


NumberToText.m:

```
#import "NumberToText.h"
#define DIGIT_LENGTH 19
#define TENS_LENGTH 9
#define MODIFIER_LENGTH 7

NSString * digitName(NSInteger number) {
switch (number) {
case 1: return @"one";
case 2: return @"two";
case 3: return @"three";
case 4: return @"four";
case 5: return @"five";
case 6: return @"six";
case 7: return @"seven";
case 8: return @"eight";
case 9: return @"nine";
case 10: return @"ten";
case 11: return @"eleven";
case 12: return @"twelve";
case 13: return @"thirteen";
case 14: return @"fourteen";
case 15: return @"fifteen";
case 16: return @"sixteen";
case 17: return @"seventeen";
case 18: return @"eighteen";
case DIGIT_LENGTH: return @"nineteen";
default: return @"";
}
}

NSString * tensName(NSInteger number) {
switch (number) {
case 0: return @"";
case 1: return @"ten";
case 2: return @"twenty";
case 3: return @"thirty";
case 4: return @"fourty";
case 5: return @"fifty";
case 6: return @"sixty";
case 7: return @"seventy";
case 8: return @"eighty";
case TENS_LENGTH: return @"ninety";
default: return @"";
}
}

NSString *

Solution

+ (NSString *)tripleAsText:(NSInteger)value;


We've declared this method in our .h file. Our .h file is for publicly available methods. We've implemented this method to provide some convenience to what we're actually trying to do with the other methods. Do we really want/need to make this particular method public? If not, simply remove its declaration from the .h file.

#define DIGIT_LENGTH 19
#define TENS_LENGTH 9
#define MODIFIER_LENGTH 7


There is very little reason to use #define directives in Objective-C (and most languages, as far as I know). There might be some scenarios where a #define is necessary and good, but for simple constant values, we shouldn't do this.

Instead, we should use constants:

const NSInteger kDigitLength = 19;
const NSInteger kTensLength = 9;
const NSInteger kModifierLength = 7;


case MODIFIER_LENGTH: return @"sextillion";


The largest possible NSInteger value is just over 9 quintillion. Even if we modify the method to take an NSUInteger, the largest value is just over 18 quintillion. We will never have a sextillion value.

- (instancetype)initWithInteger:(NSInteger)number {
    return [self initWithInteger:number negativeSign:NULL];
}


When we want to pass a null value for an argument expecting an Objective-C object, we should use nil rather than NULL. I don't think there's an actual difference, just is just Objective-C convention. However, I'd argue that we should actually choose a default string to signify negative values such as @"negative". This method is most likely to be used by people who want to just use the default string, not by people who don't want to differentiate between negative and non-negative numbers.

int subhun = value % 100;
long hun = value / 100;


subhun and hun? What are these? Longer methods names that actually tell Mr. Maintainer what the variable represent doesn't make your program slower or make it use more run time memory.

However, dividing an NSInteger and saving the result as a long WILL make your program use more memory on any 32-bit system.

NSInteger is a typedef which serves as a 32-bit integer on 32-bit systems and a 64-bit integer on 64-bit systems. So on a 32-bit system, NSInteger is an int, and on 64-bit systems, it's a long. Saving the result of value % 100 to an int is fine--we don't lose precision because we know this value is always less than 100. Saving the result of value / 100 to a long means on a 32-bit system, we're now using 8 bytes to represent a number that is definitely smaller than 4 bytes. Instead, we should just:

NSInteger hun = value / 100;


Except, we still need to have a better variable name.

[string appendString:digitName(hun)];
[string appendString:@" hundred"];
if (subhun > 0) {
    [string appendString:@" and "];
}


We can make this slightly better.

First, the first two lines should definitely be combined into:

[string appendFormat:@"%@ hundred", digitName(hun)];


But with a ternary operator, we can do it all in one operation:

[string appendFormat:@"%@ hundred%@", digitName(hun), (subhun > 0) ? @" and " : @""];


if (units>0) {
    [string appendString:@" "];
}
[string appendString:digitName(units)];


Since we know that the result of digitName(units) when units>0 evaluates to false is an empty string, we can just do this:

if (units > 0) {
    [string appendFormat:@" %@", digitName(units)];
}


Not only does this eliminate a call to appendFormat:/appendString:, it also eliminates useless calls to digitName().

if (number < 0) {
    if (!negativeSign) {
        negativeSign = @"negative";
    }
    number = ABS(number);
}
else {
    negativeSign = nil;
}

if (negativeSign) {
    [string appendString:negativeSign];
    [string appendString:@" "];
}


Remember my comment about sending @"negative" rather than nil as the default value for the negative sign? It eliminates this sort of stuff.

We start by checking whether or not the number is negative, then use this information to set the value of negativeSign, then check whether or not negativeSign is set... which we already knew from the previous if/else block...

So, keeping in mind that the user is sending us a value for negativeSign, why don't we refactor this a bit:

if (number < 0) {
    [string appendFormat:@"%@ ", negativeSign];
}


I've also eliminated the call to the ABS() macro, because, well, it's not needed. The reason I know it's not needed is because your code works for LONG_MIN and for NSIntegerMin... however the result of ABS(NSIntegerMin) is: -9223372036854775808. The actual absolute value of this number can't be represented as an NSInteger... so apparently ABS() just leaves it as is. And despite this, despite number being negative all the way through your function, it still produces the correct results... so just get rid of this line.

```
if

Code Snippets

+ (NSString *)tripleAsText:(NSInteger)value;
#define DIGIT_LENGTH 19
#define TENS_LENGTH 9
#define MODIFIER_LENGTH 7
const NSInteger kDigitLength = 19;
const NSInteger kTensLength = 9;
const NSInteger kModifierLength = 7;
case MODIFIER_LENGTH: return @"sextillion";
- (instancetype)initWithInteger:(NSInteger)number {
    return [self initWithInteger:number negativeSign:NULL];
}

Context

StackExchange Code Review Q#59546, answer score: 7

Revisions (0)

No revisions yet.