Search code examples
objective-ciosmethodscomputer-sciencecode-reuse

Is using a working method to implement others good programming practice?


I have been working on implementing the first 6 functions of the NSComparisonMethods protocol in Obj-C. I am implementing them for a Fraction object which has the following @interface

@interface Fraction : NSObject <NSComparisonMethods> {
    int numerator;
    int denominator;
}
@property int numerator, denominator;
+(int)addCounter;
-(Fraction *)addWithCount:(Fraction *)f;
-(void)print;
-(void)setTo:(int)n over:(int)d;
-(double)convertToNum;
-(int)gcd:(int)d1:(int)d2;
-(int)compare:(Fraction *)f;
// NSComparisonMethods Protocol Methods
-(BOOL)isEqualTo:(Fraction *)f;
-(BOOL)isLessThanOrEqualTo:(Fraction *)f;
-(BOOL)isLessThan:(Fraction *)f;
-(BOOL)isGreaterThan:(Fraction *)f;
-(BOOL)isGreaterThanOrEqualTo:(Fraction *)f;
-(BOOL)isNotEqualTo:(Fraction *)f;
@end

Now, the function that I have implemented from the protocol is isEqualTo. Is it good programming practice to simply use one, or in this case two working methods to implement the rest?

My implementation for those methods is as follows:

// NSComparisonMethods Protocol Methods
-(BOOL)isEqualTo:(Fraction *)f{
    [self reduce];
    [f reduce];
    return ((self.numerator==f.numerator)&&(self.denominator==f.denominator));
}
-(BOOL)isLessThan:(Fraction *)f{
    return ([self compare:f]==-1);
}
-(BOOL)isLessThanOrEqualTo:(Fraction *)f{
    return ( ([self compare:f]==-1) || ([self isEqualTo:f]) );
}
-(BOOL)isGreaterThan:(Fraction *)f{
    return ( [self compare:f]==1 );
}
-(BOOL)isGreaterThanOrEqualTo:(Fraction *)f{
    return ( ([self compare:f]==1) || ([self isEqualTo:f]) );
}
-(BOOL)isNotEqualTo:(Fraction *)f{
    return (![self isEqualTo:f]);
}

And my compare function is as follows:

-(int)compare:(Fraction *)f{
    int value = [self gcd:denominator :f.denominator];
    if ([self isEqualTo:f]) {
        return 0;
    } else if ((numerator * (f.denominator/value))>(f.numerator * (denominator/value))) {
        return 1;
    } else {
        return -1;
    }
}

All these functions are kind of redundant and I am sure there are different ways to implement them. I remember that back in the day professors used to make students do everything as if it were from scratch, but, doesn't that go against the reusing code mentality we all should have?


Solution

  • You should be reusing as much as possible.

    Put all your meat into compare, something like this:

    -(NSComparisonResult)compare:(Fraction *)f{
        float myValue = ((float)numerator / denominator);
        float theirValue = ((float)f.numerator / f.denominator);
        if (myValue == theirValue) {
            return NSOrderedSame;
        } else if (myValue > theirValue) {
            return NSOrderedDescending;
        } else {
            return NSOrderedAscending;
        }
    }
    

    I wasn't sure what you were trying to do in your compare, so I wrote one based on what I thought you should be trying to do. To compare fractions, all you need to do is resolve them to a float. The simplest way to do that should be numerator/denominator. There should be no need to find the greatest common denominator, since 1/4 = 0.25 and 2/8 = 0.25. If I've oversimplified, sorry. If you allow functions to be created with a denominator of 0 in particular (you probably shouldn't), you'll need to guard against a division by zero here.

    Then use compare to implement the others, if you need them at all:

    -(BOOL)isEqualTo:(Fraction *)f{
        return ([self compare:f] == NSOrderedSame);
    }
    -(BOOL)isLessThan:(Fraction *)f{
        return ([self compare:f] < NSOrderedSame);
    }
    -(BOOL)isLessThanOrEqualTo:(Fraction *)f{
        return ([self compare:f] <= NSOrderedSame);
    }
    -(BOOL)isGreaterThan:(Fraction *)f{
        return ([self compare:f] > NSOrderedSame);
    }
    -(BOOL)isGreaterThanOrEqualTo:(Fraction *)f{
        return ([self compare:f] >= NSOrderedSame);
    }
    -(BOOL)isNotEqualTo:(Fraction *)f{
        return ([self compare:f] != NSOrderedSame)
    }