CODEWITHSUNDEEP
swift
Vermaji
Vermaji

Reputation: 75

Error while creating a closure inside a function

Thanks a lot for the reply. It's true that I am learning to write closures at this stage. Actually I read somewhere about the "forEach" function that works on collections and that takes a single argument as parameter (i.e a closure).

Syntax of forEach is "Void forEach(body: (Int) throws -> Void) rethrows"

What I am trying to do is write a similar function with a single parameter (i.e a closure) that will calculate the Factorial of a number and so we can print the Factorial of that number. I don't want to pass that number as a second parameter to that function.

I understand that forEach is a member function of Collections class that works on each element one by one. So it picks up the elements from within the array. Similarly I have created a private property (factorialNumber) inside my class (whose value I can set using the public function "setFactorialNumber"). Now I am trying to create a public function (factorial) for my class that will have only one parameter (i.e the closure) that will use the value of "factorialNumber" property internally and calculate the factorial of that number that we can print from outside when we call that function from the other code.

Below is my class..

public class MyArray {
    private var factorialNumber = 0

    public func setFactorialNumber(factorialNumber value: Int) {
        factorialNumber = value
    }

    public func factorial(body closure: (Int) -> Void) -> Void {
        var outputString: String?
        var result = 1

        if factorialNumber <= 0 {
            outputString = nil
        } else {
            outputString = ""
            while(factorialNumber >= 1) {
                if factorialNumber == 1 {
                    outputString = outputString! +  "\(factorialNumber) = \(result)"
                    break
                } else {
                    outputString = outputString! + "\(factorialNumber) x "
                }
                result = result * factorialNumber
                factorialNumber -= 1
            }
        }
    }
}

Upvotes: 0

Views: 156

Answers (2)

staticVoidMan
staticVoidMan

Reputation: 20234

A closure is basically a function that wants to be called.
In your case, you're defining it but you're not using it.

Problems I see in factorial(body:):

  1. You should execute the closure atleast once.
    i.e. do the following at some point/s:

    closure(someValue)

  2. You seem to maintain an outputString, which looks like the thing you want the closure to take.
    In this case, your closure should take a String instead of an Int.

    func factorial(body closure: (String?) -> Void) -> Void { //...

Finally, this is what the function should look like:

func factorial(body closure: (String?) -> Void) -> Void {
    var outputString: String?

    //...your factorial logic as:
    if factorialNumber <= 0 {
        outputString = nil
    } else {
        outputString = ""
        while(factorialNumber >= 1) {
            if factorialNumber == 1 {
                outputString = outputString! +  "\(factorialNumber) = \(result)"
                break
            } else {
                outputString = outputString! + "\(factorialNumber) x "
            }
            result = result * factorialNumber
            factorialNumber -= 1
        }
    }

    //Finally, the closure call
    closure(outputString)
}

And looking at your structure, it's usage will be:

let factorial = MyArray()
factorial.setFactorialNumber(factorialNumber: 20)
factorial.factorial { (result) in
    print(result)
}

It's output is:

20 x 19 x 18 x 17 x 16 x 15 x 14 x 13 x 12 x 11 x 10 x 9 x 8 x 7 x 6 x 5 x 4 x 3 x 2 x 1 = 2432902008176640000

NOTE: Your class is Crashable

The answer above is sufficient for the particular context of your question but duly note:

  1. Your logic uses Int, so numbers higher than 20 cause your code to crash

    Solution:

    • Use Double
    • Guard your logic from crashing on inputs too large to handle

  2. If you don't setFactorialNumber(factorialNumber:), and directly jump to calling factorial(body:), your code will crash.

Improvement?:

  • Change classname from MyArray to FactorialFinder or something

  • Add an init to this class:

    init(with number: Int) {
    self.factorialNumber = number
}

  • Now that we have a custom init, make the default init private:

    private init() {}
    • So something like this is not possible FactorialFinder()
    • And FactorialFinder(with:) becomes the only way to initialize this class

Upvotes: 1

zneak
zneak

Reputation: 138031

Your first example is a good hint of your problem:

factorial {
 print("Factorial is: \($0)")
}

What is the exact text that you want this function to print? "Factorial is: 100", "Factorial is: 200"?

This is a trick question, because the answer is unknowable: your function doesn't accept a number to calculate the factorial from, it only accepts a trailing closure that defines what to do with the result.

Upvotes: 0

Related Questions