Print out statements show me that I've successfully managed to bring back the largest value in the List as the head (which was what I wanted) but the function still returns back the original head in the list. I'm guessing this is because the reference for xs is still referring to the original xs list, problem is I can't override that because it's a val.

(OK, my original version was ever so slightly more minimal but I wrote that in Scheme which doesn't have Option or type safety etc.) It doesn't need an accumulator or a local helper function because it compares the first two items on the list and discards the smaller, a process which - performed recursively - inevitably leaves you with a list of just one element which must be bigger than all the rest.

OK, why your original solution doesn't work... It's quite simple: you do nothing with the return value from the recursive call to max. All you had to do was change the line

max(remaining)

to

largest = max(remaining)

and your function would work. It wouldn't be the prettiest solution, but it would work. As it is, your code looks as if it assumes that changing the value of largest inside the recursive call will change it in the outside context from which it was called. But each new call to max creates a completely new version of largest which only exists inside that new iteration of the function. Your code then throws away the return value from max(remaining) and returns the original value of largest, which hasn't changed.

Another way to solve this would have been to use a local (inner) function after declaring var largest. That would have looked like this:

Generally, though, it is better to have recursive functions be entirely self-contained (that is, not to look at or change external variables but only at their input) and to return a meaningful value.

When writing a recursive solution of this kind, it often helps to think in reverse. Think first about what things are going to look like when you get to the end of the list. What is the exit condition? What will things look like and where will I find the value to return?

If you do this, then the case which you use to exit the recursive function (by returning a simple value rather than making another recursive call) is usually very simple. The other case matches just need to deal with a) invalid input and b) what to do if you are not yet at the end. a) is usually simple and b) can usually be broken down into just a few different situations, each with a simple thing to do before making another recursive call.

If you look at my solution, you'll see that the first case deals with invalid input, the second is my exit condition and the third is "what to do if we're not at the end".

In many other recursive solutions, Nil is the natural end of the recursion.

This is the point at which I (as always) recommend reading The Little Schemer. It teaches you recursion (and basic Scheme) at the same time (both of which are very good things to learn).

It has been pointed out that Scala has some powerful functions which can help you avoid recursion (or hide the messy details of it), but to use them well you really do need to understand how recursion works.

Excellent answer thank you and your solution is concise. The second is probably more what I'm looking for; I would opt for something more readable than fewer lines of code. I'll take a look at Scheme. I like the idea of approaching recursion by thinking about the result and working backwards from that. You more clearly know what your goal is I guess than your starting point so it makes sense to start from there.
–
James MurphySep 18 '13 at 12:58

@Dropkick Also, I think my concise version is more readable to anybody who understands recursion (and probably even to many who don't). The 3 case statements map exactly onto the 3 possible states of execution. One of the many problems with imperative style is that it is verbose and requires careful reading both to understand what is actually being attempted and to catch bugs. Go with what works for you, sure :) but I promise you there's value in considering this approach.
–
itsbruceSep 18 '13 at 13:30

At least consider using case expressions and pattern matching as the basic template for your recursive functions, even if what you do with the matched patterns is more imperative in style. And now I will stop preaching ;)
–
itsbruceSep 18 '13 at 13:41

Thanks for the response Diego. This is slightly simpler than my above solution and more readable. I'm not used to having to write recursive functions to solve these problems so I'll put it down to my non-functional brain trying to figure this out
–
James MurphySep 17 '13 at 21:43

The following is a typical way to solve this sort of problem. It uses an inner tail-recursive function that includes an extra "accumulator" value, which in this case will hold the largest value found so far:

Why not make the first element the minimum value? Do you really want max([]) = Int.MinValue?
–
kqrSep 17 '13 at 21:54

@kqr Yes, that would be an improvement - except: what to return if the list is empty?
–
ShadowlandsSep 17 '13 at 21:55

@Shadowlands None, if I get to choose freely. Throw an exception is another alternative which is worse than None, but better than an error code.
–
kqrSep 17 '13 at 21:56

1

I'm unsure as to why this has had upvotes. The question wasn't around how to write the most minimal possible code to solve the solution but write something that is human-readable. One of the key principles of clean code is that your code should be understandable within a minute or so. Some of these answers are not and I doubt many developers would understand what said code is doing a few months down the line.
–
James MurphySep 18 '13 at 12:53

Kinda glad we have loops - this is an excessively complicated solution and hard to get your head around in my opinion. I resolved the problem by making sure the recursive call was the last statement in the function either that or xs.head is returned as the result if there isn't a second member in the array.

Don't lose heart. It's a different paradigm and will cause frustration initially. While this was a good exercise, so you know a more idiomatic solution to this problem is to use reduceLeft. Specifically, xs.reduceLeft((maxThusFar, next) => if(next > maxThusFar) next else maxThusFar). You may find my recent blog post on this topic helpful.
–
joesciiSep 17 '13 at 21:54

2

To expand on the comment by @barnesjd: Contrary to popular belief, writing recursive functions is not actually that common in functional programming. Sometimes it makes the code clearer, but a lot of times it just makes the code more difficult to read. The functional solution is often to use special loops depending on what you want to do, like reduceLeft or map or filter or forever. In functional languages you are not limited to two or three kinds of loops – there exists a ton of them and you can even invent your own!
–
kqrSep 17 '13 at 21:59

1

@barnesjd By the way, you could also make that xs.reduceLeft(math.max) if you recall that the lambda you wrote is the definition of math.max(a, b).
–
kqrSep 17 '13 at 22:29