Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

First, the struct itself:

struct Fraction {
    var numerator: Int
    var denominator: Int {
    didSet (oldDenominator) {
        if self.denominator == 0 {
            self.denominator = oldDenominator
        }
    }
    }

    init() {
        self.init(numerator: 0, denominator: 1)
    }

    init(numerator: Int) {
        self.init(numerator: numerator, denominator: 1)
    }

    init(reciprocalOf denominator: Int) {
        self.init(numerator: 1, denominator: denominator)
    }

    init(numerator: Int, denominator: Int) {
        self.numerator = numerator;
        self.denominator = denominator;
    }

    mutating func reduce() {
        let gcd = greatestCommonDenominator(self.numerator,self.denominator)
        self.numerator /= gcd
        self.denominator /= gcd
    }

    func fraction() -> (numerator:Int,denominator:Int) {
        return (self.numerator,self.denominator)
    }
}

The GCD function the struct's reduce() function uses:

func greatestCommonDenominator(first: Int, second: Int) -> Int {
    return second == 0 ? first : greatestCommonDenominator(second, first % second)
}

An extension on Int to conveniently make some of these fractions:

extension Int {
    var fraction: Fraction {
        return Fraction(numerator: self)
    }
    var reciprocalOf: Fraction {
        return Fraction(reciprocalOf: self)
    }
}

And now for the operators. First, your basic add, subtract, multiply, and divide:

@infix func + (first: Fraction, second: Fraction) -> Fraction {
    let numerator = (first.numerator * second.denominator) + (first.denominator * second.numerator)
    let denominator = (first.denominator * second.denominator)
    var frac = Fraction(numerator: numerator, denominator: denominator)
    frac.reduce()
    return frac
}

@infix func - (first: Fraction, second: Fraction) -> Fraction {
    let subtractor = Fraction(numerator: -second.numerator, denominator: second.denominator)
    return first + subtractor
}

@infix func * (first: Fraction, second: Fraction) -> Fraction {
    let numerator = first.numerator * second.numerator
    let denominator = first.denominator * second.denominator
    var frac = Fraction(numerator: numerator, denominator: denominator)
    frac.reduce()
    return frac
}

@infix func / (first: Fraction, second: Fraction) -> Fraction {
    let divisor = Fraction(numerator: second.denominator, denominator: second.numerator)
    return first * second
}

I don't think the modulo operator makes sense here, given we're dealing with fractions. Is there an operator I'm missing?

Now, the compound assignment operators:

@assignment func += (inout left: Fraction, right: Fraction) {
    left = left + right
}

@assignment func -= (inout left: Fraction, right: Fraction) {
    left = left - right
}

@assignment func *= (inout left: Fraction, right: Fraction) {
    left = left * right
}

@assignment func /= (inout left: Fraction, right: Fraction) {
    left = left / right
}

I actually don't know if these are strictly necessary, or if Swift will allow these and assume left = left + right to be left += right and overloading is only necessary if you need something custom. I'm not sure.

I also wrote functions for doing math with a fraction and an integer:

@infix func + (fraction: Fraction, integer: Int) -> Fraction {
    return fraction + integer.fraction
}

@infix func - (fraction: Fraction, integer: Int) -> Fraction {
    return fraction - integer.fraction
}

@infix func * (fraction: Fraction, integer: Int) -> Fraction {
    return fraction * integer.fraction
}

@infix func / (fraction: Fraction, integer: Int) -> Fraction {
    return fraction / integer.fraction
}

@infix func + (integer: Int, fraction: Fraction) -> Fraction {
    return integer.fraction + fraction
}

@infix func - (integer: Int, fraction: Fraction) -> Fraction {
    return integer.fraction + fraction
}

@infix func * (integer: Int, fraction: Fraction) -> Fraction {
    return integer.fraction * fraction
}

@infix func / (integer: Int, fraction: Fraction) -> Fraction {
    return integer.fraction / fraction
}

What can be improved? What's missing?

share|improve this question
1  
I would explore having your init function return an optional Fraction, with a nil value when the denominator is 0. Fraction(numerator:1, denominator:0) should NOT just silently return a Fraction object that is invalid and will cause trouble later. Swift's optionals seem tailor-made to handle this situation. –  GraniteRobert Jul 13 at 12:14

1 Answer 1

I think everything looks quite nice here. It runs well, and you run the necessary checks to make sure that everything is safe.

I did notice that you went to great extent to implement a bunch of operators for your Fraction structure. But you forgot to implement the common prefix and postfix operators. Say I want to do this to your structure:

var test = Fraction(numerator: 3, denominator: 5)
test++

The Swift Playground environment will throw this error at me:

Playground execution failed: error: <EXPR>:137:1: error: 'Fraction' is not identical to 'Float'
test++

You can combine the @assignment attribute with either the @prefix or @postfix attribute, as in this implementation of the postfix increment operator (++a) for Fraction instances:

@postfix @assignment func ++ (inout frac: Fraction) -> Fraction {
    frac += 1.fraction
    return frac
}

Now the Playground environment doesn't complain at me for using my much-loved common operator. I'll leave it up to you to implement the rest of them (since they aren't too hard to implement ;)).

share|improve this answer
    
Yeah. Those definitely ought to be added. –  nhgrif Jul 12 at 23:52

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.