Recursive function not working as planned

Question

I am writing a function in Scheme that is supposed to take two integers, X and Y, and then recursively add X/Y + (X-1)/(Y-1) + ...until one of the numbers reaches 0.

For example, take 4 and 3:

4/3 + 3/2 + 2/1 = 29/6

Here is my function which is not working correctly:

(define changingFractions (lambda (X Y)
    (cond 
        ( ((> X 0) and (> Y 0)) (+ (/ X Y) (changingFunctions((- X 1) (- Y 1)))))
        ( ((= X 0) or (= Y 0)) 0)
    )
))

EDIT: I have altered my code to fix the problem listed in the comments, as well as changing the location of or and and.

(define changingFractions (lambda (X Y)
    (cond 
        ( (and (> X 0) (> Y 0)) (+ (/ X Y) (changingFunctions (- X 1) (- Y 1) )))
        ( (or (= X 0) (= Y 0)) 0)
    )
))

Unfortunately, I am still getting an error.

When something does not "work" correctly, please provide an error message. Here, I guess that `(- x 1)` not being a function is causing a problem. You should try `(changingFunctions (- x 1) (- y 1))` instead of enclosing arguments in parenthesis. — coredump, Feb 04 '16 at 15:41
Do you really need to check that both X and Y are zero if your first test fails? If any of your arguments is negative, the return value of your function is unspecified per cond. — coredump, Feb 04 '16 at 15:44
@coredump- The question is to be completed in an app online. [Here](http://imgur.com/QCRSpYZ) is the error is it giving me. Your first suggestion makes perfect sense and I have adjusted my code, but it is still somehow wrong. I have tried your second suggestion too, which also doesn't solve the problem. I understand that there is no need for it, but I like to have the base case visible. — KOB, Feb 04 '16 at 15:50
@coredump- My goodness, how did I not spot that in over an hour of trying to solve this! Everything is now working as it should. Thank you! — KOB, Feb 04 '16 at 15:58
"Unfortunately, I am still getting an error." It would have helped to have shown that error in the question... — Joshua Taylor, Feb 04 '16 at 17:17

jkiiski · Accepted Answer · 2016-02-04T16:36:46.823

5

A couple of problems there:

You should define a function with the syntax (define (func-name arg1 arg2 ...) func-body), rather than assigning a lambda function to a variable.
The and and or are used like functions, by having them as the first element in a form ((and x y) rather than (x and y)). Not by having them between the arguments.
You have an extra set of parens around the function parameters for the recursive call, and you wrote changingFunctions when the name is changingFractions.
Not an error, but don't put closing parens on their own line.
The naming convention in Lisps is to use dashes, not camelcase (changing-fractions rather than changingFractions).

With those fixed:

(define (changing-fractions x y)
  (cond 
   ((and (> x 0) (> y 0)) (+ (/ x y) (changing-fractions (- x 1) (- y 1))))
   ((or (= x 0) (= y 0)) 0)))

But you could change the cond to an if to make it clearer:

(define (changing-fractions x y)
  (if (and (> x 0) (> y 0))
      (+ (/ x y) (changing-fractions (- x 1) (- y 1)))
      0))

edited Feb 04 '16 at 16:36

answered Feb 04 '16 at 16:01

jkiiski

8,206
2
28
44

Thank you for all of the corrections. The only issue I don't understand is when/when not to use a lambda function? – KOB Feb 04 '16 at 16:43
`(define (func p1 p2 ...) ...)` is a shorthand for `(define func (lambda (p1 p2 ...) ...))`. – uselpa Feb 04 '16 at 16:54
@CornOnTheKob Functions made with `lambda` are usually used in an ad hoc manner when you need a function for higher order functions (`map` and such) and the function is either too specific or too trivial to be named, or if the function needs to use variables from the current scope (closure). – jkiiski Feb 04 '16 at 17:03
You also use lambdas when you want to delay evaluation. – WorBlux Feb 04 '16 at 17:07

Mulan · Answer 2 · 2016-02-05T14:00:42.090

I personally like this implementation. It has a proper tail call unlike the other answers provided here.

(define (changing-fractions x y (z 0))
  (cond ((zero? x) z)
        ((zero? y) z)
        (else (changing-fractions (sub1 x) (sub1 y) (+ z (/ x y))))))

(changing-fractions 4 3) ; => 4 5/6

The trick is the optional z parameter that defaults to 0. Using this accumulator, we can iteratively build up the fractional sum each time changing-fractions recurses. Compare this to the additional stack frames that are added for each recursion in @jkliski's answer

; changing-fractions not in tail position...
(+ (/ x y) (changing-fractions (- x 1) (- y 1)))

Recursive function not working as planned

2 Answers2