Merge two classes in one class

Question

I have two classes: 1. Fraction class: numerator and denominator AND 2. OperationWithFractions class which performs a calculation like adding two fractions and represents the result as a String fraction (for example 32 / 5).

It works, but I would like to simplify by merging both classes in only one, so all properties, methods and initializers would be under the same umbrella.

class Fraction {
    var numerator = 0
    var denominator = 0

    init (_ numer: Int, _ denom: Int){
        self.numerator = numer
        self.denominator = denom
    }
}

class OperationWithFractions {
    var fraction1: Fraction
    var fraction2: Fraction

    init(_ fraction1: Fraction, _ fraction2: Fraction) {
        self.fraction1 = fraction1
        self.fraction2 = fraction2
    }

    func addFractions()->String {
        var result = ""

        result = "\(fraction1.numerator * fraction2.denominator + fraction1.denominator * fraction2.numerator) / \(fraction1.denominator * fraction2.denominator)"

        return result
    }
}

class ViewController: UIViewController {
    override func viewDidLoad() {
        super.viewDidLoad()

        let result = OperationWithFractions(Fraction(5, 10), Fraction(10, 20)).addFractions()
        print(result)

        let result2 = OperationWithFractions(Fraction(10, 2), Fraction(8, 2)).addFractions()
        print(result2)
    }
}

Answer 1

class OperationWithFractions {
    typealias Fraction = (numerator: Int, denominator: Int)

    private let fraction1: Fraction
    private let fraction2: Fraction

    init(_ fraction1: Fraction, _ fraction2: Fraction) {
        self.fraction1 = fraction1
        self.fraction2 = fraction2
    }

    func addFractions() -> String {
        return "\(fraction1.numerator * fraction2.denominator + fraction1.denominator * fraction2.numerator) / \(fraction1.denominator * fraction2.denominator)"
    }
}

let fraction1 = OperationWithFractions.Fraction(numerator: 1, denominator: 2)
let fraction2 = OperationWithFractions.Fraction(numerator: 2, denominator: 3)

let operation = OperationWithFractions(fraction1, fraction2)
print(operation.addFractions()) // 7/6

Now, what is going on here?

I converted your entire fraction class into a typealias since it exists solely to format your incoming data.

Next, I replaced your var fraction1: Fraction with private let fraction1: Fraction. This encapsulates logic that only your class needs.

The init function is unchanged.

Finally, I cleaned up your return since there is no need for the initial variable being set and a separate return.

It is also possible to just create an initializer that accepts num1, den1, num2, den2, etc... but this will not scale well should you want to add 3 or more fractions.

Answer 2

Addition of two fractions should return a Fraction, not a string. The conversion to strings is only done if you need a textual representation (e.g. for printing the final result).

Arithmetic operations can be implemented as operators of the fraction type:

struct Fraction {
    // ...

    static func +(lhs: Fraction, rhs: Fraction) -> Fraction { ... }
}

That allows you to write

let result = Fraction(1, 2) + Fraction(-1, 6)

Also prefer values types over reference types, and constant properties over variable properties. A first implementation could be

struct Fraction {
    let numerator: Int
    let denominator: Int

    init (_ numer: Int, _ denom: Int) {
        self.numerator = numer
        self.denominator = denom
    }

    static func +(lhs: Fraction, rhs: Fraction) -> Fraction {
        return Fraction(lhs.numerator * rhs.denominator + lhs.denominator * rhs.numerator,
                        lhs.denominator * rhs.denominator)
    }
}

The CustomStringConvertible protocol is adopted to provide the textual representation:

extension Fraction: CustomStringConvertible {
    var description: String {
        return "\(numerator)/\(denominator)"
    }
}

This already works

// Compute 1/2 - 1/6 + 1/3:
let result = Fraction(1, 2) + Fraction(-1, 6) + Fraction(1, 3)
print(result) // 24/36

but the result is not completely satisfying because it is not reduced to the lowest common terms. Also

 print(Fraction(2, -3)) // 2/-3

is not optimal.

Here is a slightly more sophisticated version where the results are reduced to the lowest terms. Apart from the gcd utility function, everything is defined inside the Fraction type.

// Greatest common divisor
func gcd(_ a : Int, _ b : Int) -> Int {
    var (a, b) = (a, b)
    while b != 0 {
        (a, b) = (b, a % b)
    }
    return a
}

struct Fraction {
    let numerator: Int
    let denominator: Int

    init (_ numer: Int, _ denom: Int, reduce: Bool = false) {
        if reduce {
            let commonFactor = gcd(numer, denom)
            self.numerator = numer / commonFactor
            self.denominator = denom / commonFactor
        } else {
            self.numerator = numer
            self.denominator = denom
        }
    }

    static func +(lhs: Fraction, rhs: Fraction) -> Fraction {
        return Fraction(lhs.numerator * rhs.denominator + lhs.denominator * rhs.numerator,
                        lhs.denominator * rhs.denominator, reduce: true)
    }
}

extension Fraction: CustomStringConvertible {
    var description: String {
        if denominator == 1 {
            return "\(numerator)"
        } else if denominator < 0 {
            return "\(-numerator)/\(-denominator)"
        } else {
            return "\(numerator)/\(denominator)"
        }
    }
}

Example usage:

// Compute 1/2 - 1/6 + 1/3:
let result = Fraction(1, 2) + Fraction(-1, 6) + Fraction(1, 3)
print(result) // 2/3

Now you can add other operators (e.g. -, *, /), error checking (e.g. for a zero denominator, or integer overflow), additional instance methods (e.g. “absolut value”), etc.

Answer 3

It looks like you want to avoid defining the two properties and initializer in both classes. That's easy enough; just use generics, like

class MathematicalOperation<T> {  // you can limit T to numerics, but there's no advantage here
    var firstOperand: T
    var secondOperand: T

    init (_ first: T, _ second: T){
        firstOperand = first
        secondOperand = second
    }
}

Then you can subclass this to make your Fraction and OperationWithFractions subclasses:

class Fraction: MathematicalOperation<Int> {
    // no need to define another initializer or properties
}

class OperationWithFractions: MathematicalOperation<Fraction> {
    func addFractions()->String {
        return "\(firstOperand.firstOperand * secondOperand.secondOperand + firstOperand.secondOperand * secondOperand.firstOperand) / \(firstOperand.secondOperand * secondOperand.secondOperand)"
    }
}

However, this is obviously less than ideal because firstOperand.firstOperand, secondOperand.firstOperand, etc. aren't very clear. And therein lies your problem: you're trying to save yourself some writing, but in the process, you've muddled the problem you're solving because you've had to pick a generic-enough name for the properties. In that case, it hardly seems worth doing this. You'd have been better off the way you were before, where the property names meant something.