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

Fraction class implemented in Objective-C

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

Problem

Inspired by this question: Fraction (rational number) structure with custom operators, I have written this class for doing some simple work with fractions.

Fraction.h:

#import 

@interface Fraction : NSObject

@property int numerator, denominator;

-(void) print;
-(void) setNumerator: (int) n;
-(void) setDenominator: (int) d;
-(void) setTo: (int) n over: (int) d;
-(double) convertToNum;
-(void) add: (Fraction*) f;
-(void) subtract: (Fraction*) f;
-(void) multiply: (Fraction*) f;
-(void) divide: (Fraction *) f;
-(void) reduce;

@end


Fraction.m:

#import "Fraction.h"

@implementation Fraction

@synthesize numerator, denominator;

-(void) print
{
    NSLog(@"%i/%i", numerator, denominator);
}

-(void) setNumerator: (int) n
{
    numerator = n;
}

-(void) setDenominator: (int) d
{
    denominator = d;
}

-(void) setTo: (int) n over: (int) d
{
    numerator = n;
    denominator = d;
}

-(double) convertToNum
{
    if (denominator) return (double) numerator / denominator;
    else return NAN;
}

-(void) add: (Fraction*) f
{
    numerator = numerator * f.denominator + denominator * f.numerator;
    denominator *= f.denominator;

    [self reduce];
}

-(void) subtract: (Fraction*) f
{
    numerator = numerator * f.denominator - denominator * f.numerator;
    denominator *= f.denominator;

    [self reduce];
}

-(void) multiply: (Fraction *) f
{
    numerator *= f.numerator;
    denominator *= f.denominator;

    [self reduce];
}

-(void) divide: (Fraction *) f
{
    numerator *= f.denominator;
    denominator *= f.numerator;

    [self reduce];
}

-(void) reduce
{
    int u = numerator;
    int v = denominator;
    int temp;

    while (v)
    {
        temp = u % v;
        u = v;
        v = temp;
    }

    numerator /= u;
    denominator /= u;
}

@end


main.m:

```
#import "Fraction.h"

int main(int argc, const char* argv[])
{
@autoreleasepool
{
Fraction *frac1 = [[Fraction alloc] init];
Fraction *frac2 = [[Fraction alloc

Solution

@property

You declare properties in the header file, but you don't actually use them as anything more than instance variables, which suggests you may not quite understand their full power.

An Objective-C property is three things.

  • An instance variable



  • A setter



  • A getter



When you write @property int numerator;, you have created all three of these things.

Whether or not you write anything else, the class now has a private instance variable known as _numerator, a getter method in the form of - (int)numerator; and a setter method in the form of - (void)setNumerator:(int)numerator;

So given a fraction variable called myFrac, all of the following is instantly valid:

myFrac.numerator = 3;
NSLog(@"%i", myFrac.numerator);
[myFrac setNumerator: 4];
NSLog(@"%i", [myFrac numerator]);


The square bracket notation is perfectly valid, though when dealing with properties in Objective-C, we tend to prefer just using the dot notation.

What's more is that this fact makes these methods superfluous:

-(void) setNumerator: (int) n
{
    numerator = n;
}

-(void) setDenominator: (int) d
{
    denominator = d;
}


Except... not completely. The first one is definitely unnecessary. The second, however... it needs some logic. Zero is not a valid denominator. We need to prevent the case of the user setting the denominator to zero. I think this is one of the rare cases where throwing an exception in Objective-C might be okay. And in fact, why don't we just throw the exact exception that'd be thrown if we did actually divide by zero?

- (void)setDenominator:(int)denominator {
    if (denominator == 0) {
        // no reason to build a NSException object, just:
        __unused int divisionByZero = numerator/denominator;
    }
    _denominator = denominator;
}


I mean, I see that in converting it to a double, you deal with this, but what about when you print? You really shouldn't let the object exist in an invalid state.

And you might have noticed I used a mysterious _denominator there. As of Xcode 4 (I think, maybe even older), properties are autosynthesized in the .m file to their name prefixed with an underscore. The @synthesize directive is no longer needed, and all Objective-C developers I know have abandoned it in favor of using the autosynthesized version. The underscored name also is a huge clue that this is a direct access instance variable and not anything else.

-(void) setTo: (int) n over: (int) d
{
    numerator = n;
    denominator = d;
}


You follow this pattern of using short names for the arguments, and I think it's probably simply to avoid the name class with your synthesized property names. And now you've just discovered another reason why the auto-synthesized underscore prefixed property names are great! Although, truthfully, we should be going through the setters, so this method should look like this:

- (void)setTo:(int)numerator over:(int)denominator {
    self.numerator = numerator;
    self.denominator = denominator;
}


And if you put an NSLog statement in setDenominator:, you'll see that this method will now go through that method to set the value.

By the way, notice how I keep moving your opening braces to the correct line?

-(void) print
{
    NSLog(@"%i/%i", numerator, denominator);
}


As far as I know, methods like this, across the board, are a no-no in OOP. Instead, classes should have an instant method which returns a string to be printed by whoever is using the class if they want to print the class. In most languages, the method is called toString(), in Objective-C, we call it description. Your description method should look like this:

- (NSString *)description {
    return [NSString stringWithFormat:@"%i/%i",numerator,denominator];
}


One thing to note is the behavior of NSLog. NSLog doesn't just work for NSString objects. It works any object. As all Objective-C objects have a description method (which at a minimum is inherited from their super class), NSLog prints the value returned by the object's description method. As such, by implementing description, see what happens if you call:

NSLog(@"%@", myFrac);


-(double) convertToNum
{
    if (denominator) return (double) numerator / denominator;
    else return NAN;
}


This method is actually problematic for me. There's no telling what type of variable someone might need out of your fraction. Whose to say they don't need a float? Or an int? Or an NSInteger? There are a lot of possible types of number data types that one might need...

So, for starters, I'll point you to other Objective-C classes with method names like doubleValue, intValue, integerValue, etc. As such, a method that returns the double representation of an object should probably be called doubleValue.

But perhaps what may be even more important that writing a method for all the different numeric data types... why don't we m

Code Snippets

myFrac.numerator = 3;
NSLog(@"%i", myFrac.numerator);
[myFrac setNumerator: 4];
NSLog(@"%i", [myFrac numerator]);
-(void) setNumerator: (int) n
{
    numerator = n;
}

-(void) setDenominator: (int) d
{
    denominator = d;
}
- (void)setDenominator:(int)denominator {
    if (denominator == 0) {
        // no reason to build a NSException object, just:
        __unused int divisionByZero = numerator/denominator;
    }
    _denominator = denominator;
}
-(void) setTo: (int) n over: (int) d
{
    numerator = n;
    denominator = d;
}
- (void)setTo:(int)numerator over:(int)denominator {
    self.numerator = numerator;
    self.denominator = denominator;
}

Context

StackExchange Code Review Q#56883, answer score: 9

Revisions (0)

No revisions yet.