23

Okay, I was being interviewed at a company and the interviewer asked me a recursion problem. It was an online interview, so, he had set up the problem statement and a function signature on CodeSandbox (an online code editor/collaboration tool). I was supposed to fill-up the function body. He had only one parameter in the function signature. I added another parameter just to keep track of the result. He said I shouldn't add another parameter(I was providing a default value to the additional parameter), as it changes the function signature.

Now, in my opinion, if you are adding an optional parameter to the signature, it wouldn't make any difference. Let me take a simple example to make it more clear to you:

Problem: Check if the input is a palindrome.

Solution 1:

function isPalindrome(input, index = 0){
    const isAMatch = input[index] === input[input.length - 1 - index]

    if (index === Math.floor((input.length - 1) / 2)) {
        return isAMatch
    }

    if (isAMatch) {
        return isPalindrome(input, ++index)
    }

    return isAMatch
}

In the solution above, I added an optional parameter: index to keep track of the index to be matched. The question here is that if it's reasonable to add this optional parameter?

Solution 2:

function isPalindrome(str){
    if(str.length === 1) return true;
    if(str.length === 2) return str[0] === str[1];
    if(str[0] === str.slice(-1)) return isPalindrome(str.slice(1,-1))
    return false;
}

In this solution, we aren't using any additional parameters.

Now I'm repeating the question again, would Solution 1 be considered as an invalid solution?

Glorfindel
  • 3,137
  • 6
  • 25
  • 33
Bharat Soni
  • 357
  • 1
  • 2
  • 6
  • 46
    When the task is to write a function with a given signature, you should write a function with that given signature. And if the task is to write a correct function (and not a fast function) I would prefer an implementation for which it can be easier seen it is correct or not. Your solution 2 looks way simpler, and I can immediately spot that you missed to handle the case "str.len==0" correctly, so I would prefer that as a first shot (but with a correction for the missing case, of course). – Doc Brown Sep 02 '20 at 13:50
  • 3
    Solution 1 infinitely recurses given an empty string as input. Also if index >= str.length – Vaelus Sep 02 '20 at 16:26
  • 10
    @Vaelus: And the latter is precisely why the `index` parameter should be private and *not* be exposed to the caller. – Jörg W Mittag Sep 02 '20 at 20:03
  • Sounds like a frustrating interview. There are a variety of reasons to limit the inputs and outputs of a function including security, simplicity, generality, etc... – user121330 Sep 03 '20 at 20:39
  • 1
    Next time, use `arguments[1]`. – xehpuk Sep 03 '20 at 22:00
  • 1
    The question title should have been: Is it wrong to change the function signature from what was given by the interviewer? And to answer you question you could have written a different function the way you wanted and called it from the "original". – tpb261 Sep 05 '20 at 06:05
  • What you did was is called tail-recursion: https://stackoverflow.com/questions/33923/what-is-tail-recursion – ksinkar Sep 18 '20 at 09:49
  • I would have replaced `return isAMatch` with `return false`. – qwerty Sep 18 '20 at 13:41
  • It seems to me that the key here is that adding an additional parameter in JavaScript does NOT break any existing code, as long as the function behavior doesn't change for the case where the parameter is omitted. In that case "changes the function signature" is more like "adding an additional function signature". I think the interviewer is being overly pedantic – A. L. Flanagan Sep 18 '20 at 15:09

8 Answers8

79

Solution 1 is not valid, because an unexpected signature can fail in unexpected ways. You were given a particular function signature because that's how it's expected to be used.

An example of such an unexpected failure, using Solution 1:

>> ["aba", "aba"].map(isPalindrome)
== Array [ true, true ]
>> ["aba", "aba", "aba"].map(isPalindrome)
Uncaught InternalError: too much recursion

This occurs because map does use the additional arguments: the second is the index in the array.

Solution 2 does not fail like this, because it maintains the original signature and ignores the additional arguments.

This can be worked around, such as by calling isPalindrome wrapped in another function like .map(value => isPalindrome(value)), but the point is that having been given a particular signature indicates it's meant to be used in a particular way, and without knowing what that way is, you just can't really say it makes no difference.

Izkata
  • 6,048
  • 6
  • 28
  • 43
  • 6
    In addition to behavioral changes like the one you describe above, exposing internal parameters in the signature also tends to confuse consumers of the function; such parameters are often exposed to the programmer by their IDE. – Brian Sep 04 '20 at 14:26
  • using `array.map(someFunction)` is javascript is generally an antipattern - and should not be used. For example, `["1","2","3"].map(parseInt)` returns `[1, NaN, NaN]`. – Carson Graham Sep 04 '20 at 17:37
  • Thinking about it, the `parseInt` function is really similar to the `isPalindrome` function as it takes in an optional parameter with a default value as the second argument. Despite the "unusual" behavior when using `map`, `parseInt` is still part of the standard library. In addition, map passes 3 parameters, not 2, which while you didn't touch on it in your answer, if you're going to pass a function like that you need to know and have the number of arguments memorized. – Carson Graham Sep 04 '20 at 17:43
  • @CarsonGraham: Would you avoid `array.map(someFunction)` even if `someFunction` is documented to be usable that way? – ruakh Sep 04 '20 at 17:50
  • @CarsonGraham No, that's not an antipattern. The real problem is that the javascript standard library is full of antipatterns and nasty surprises. – Sulthan Sep 09 '20 at 19:24
53

It is often necessary to introduce additional parameters when turning an iterative solution into a recursive, especially into a tail-recursive one.

The reason is that the implicit state that the iterative version has must go somewhere and the only place it can go is on the call stack … either in the return value or parameters.

The way this is usually done is the same way we hide implementation details in any other case: by introducing a private implementation. In languages which support nested functions, the standard way is to introduce a nested helper function like this:

function isPalindrome(input) {
    if (input.length <= 1) {
        return true;
    }

    return isPalindromeRec();

    function isPalindromeRec(index = 0) {
        const isAMatch = input[index] === input[input.length - 1 - index]

        if (index === Math.floor((input.length - 1) / 2)) {
            return isAMatch
        }

        if (isAMatch) {
            return isPalindromeRec(++index)
        }

        return isAMatch
    }
}
Jörg W Mittag
  • 101,921
  • 24
  • 218
  • 318
  • Solution 2 is already tail recursive though. – Vaelus Sep 02 '20 at 16:19
  • 8
    And even without nested functions, a well-known trick is a _driver_ function supplying the extra parameters to the real one: `isPalind(string w) { return _isPalind(w,0); }` – Owen Reynolds Sep 02 '20 at 22:30
  • 6
    while agreeing with your approach in general, I'd change it a bit: use two parameters in the internal function, `startIndex` and `endIndex`. This would remove the need to a complicated call to `Math.floor`, making the stop condition simply `startIndex >= endIndex` – IMil Sep 03 '20 at 00:51
  • 3
    You can easily change the inner function so that it both eliminates the local variable `isAMatch` and is tail recursive. – David Hammen Sep 03 '20 at 06:25
  • 5
    @IMil: I tried to keep the code as close to the OP's as possible, only demonstrating how you can use auxiliary functions to hide the change in signature. – Jörg W Mittag Sep 03 '20 at 09:53
  • 1
    @DavidHammen: I tried to keep the code as close to the OP's as possible, only demonstrating how you can use auxiliary functions to hide the change in signature. Note that there is no change required to make it tail recursive. Both my version and the OP's original version already are. – Jörg W Mittag Sep 03 '20 at 09:54
  • Best answer IMO. I would only add the general case of using a public and a private function for this. – Blueriver Sep 04 '20 at 20:40
  • This is even better as you don't need to (and should not) make `index` an optional parameter with a default value, but you can just call `isPalindromRec(0)`. – Bergi Sep 05 '20 at 02:35
37

Well I like the index solution simply because it doesn't require creating multiple sub strings on the heap.

The problem with interview questions is they're mostly "guess what I'm thinking" games. So while you and I might be fully objectively right about which is the better solution the point is to show that you can work with the interviewer to either get them to see that or figure out what will make them happy even if it is stupid.

But to answer your exact question, no. Solution 1 is still valid. If challenged about the signature all you had to do was call _isPalindrome(input, index) from isPalindrome(input). No one said you couldn't define a new function. You are still using recursion. But can you get the interviewer to see that?

Being right is a small consolation if you don't get the job.

candied_orange
  • 102,279
  • 24
  • 197
  • 315
  • actually, I introduced the new pram just to get rid of multiple substrings. Thanks for the explanation :) – Bharat Soni Sep 02 '20 at 06:27
  • 20
    Downvoted because in many systems, most public methods are implemented to an interface, and interfaces must be written based on what their consumers need to do, not what their implementations would like. You are acting like the exact signature is a minor detail and you need to convince the interviewer of this, but it's very much not. In fact thinking it's OK to change the signature when implementing an interface is a common error I see in junior programmers and signifies they don't fully understand what they're doing yet. – JounceCracklePop Sep 03 '20 at 18:45
  • 5
    @candied_orange write it again. "I like the solution"; "get them to see that [it's right] or [do it stupid]", "Solution 1 is still valid"; "[do it the stupid way] if challenged"; "being right [by changing the signature] is a small consolation". You are very clearly saying that changing the signature is OK and even "right", when it's very much not OK and very much wrong. – JounceCracklePop Sep 03 '20 at 19:24
  • 3
    @candied_orange Yes. If your interface specifies the one-argument signature then it literally does not even compile. Even if you're not implementing an interface, you're leaking implementation details to the caller. The caller should never be allowed to specify a different value. Izkata showed how this can unexpectedly break things in another answer. – JounceCracklePop Sep 03 '20 at 19:44
  • 1
    @candied_orange look at Izkata answer (`.map(isPalindrome)`) - if OP explained that case to interviewer I'm pretty sure the solution would be accepted... But indeed adding another parameter is breaking change even in JavaScript. The question OP asked is only tangentially related to the problem - indeed one can use more parameters to recursive function... – Alexei Levenkov Sep 03 '20 at 20:36
  • 5
    @candied_orange to me your answer reads as "interviewers are generally clueless about what they are asking and this is completely pointless restriction to not allow to add extra default parameter, here is a hack to give that @#$# what they want" (which may be a reason why OP accepted the answer). I'm not trying to argue much here, it is likely my bias (as I've been on interviewer's side of the conversation more often) rather than post itself. – Alexei Levenkov Sep 03 '20 at 20:54
  • 17
    -1. There is a huge difference between "this is a correct solution" and "you can easily explain how to make a correct solution out of it". The interviewer was correct to insist that the solution changes the signature, and the OP was wrong to say that it doesn't matter. I'd expect the interviewer to accept the explanation like "well, we can add a simple wrapper to have the exact signature we need, let's not waste time on writing it explicitly", but OP clearly insisted on the default argument making no difference at all. – Frax Sep 03 '20 at 21:47
  • @candied_orange You did, your answer says: "Solution 1 is still valid." Also "Being right is a small consolation (...)" is a strong hint, though not as explicit. – Frax Sep 03 '20 at 21:52
  • 3
    Perhaps we just read the question slightly differently. For me the key sentence is "Now, in my opinion, if you are adding an optional parameter to the signature, it wouldn't make any difference". We can see from Izkata's example that it's clearly wrong. You imply the OP was right and just didn't get the message across to interviewer, but the issue is that OP's message was actually incorrect. OP shouldn't look for consolation in being right, because he wasn't. I think your answer would be way better if you ended your 3rd paragraph with "But you may need to show the interviewer you know that." – Frax Sep 03 '20 at 22:12
  • And maybe if you removed the snarky "Being right is a small consolation" remark. – Frax Sep 03 '20 at 22:13
  • _isPalindrome need not be an named function it can be local variable – Jasen Sep 05 '20 at 06:28
  • 1
    @CarlLeth: Your comment is a meaningless distinction. Implementations can differ from interfaces, as long as you can connect one to the other. Applying real life practice, OP could've developed a second submethod where he could've added any parameter he liked. That retains the signature for the already defined (interface) method. Truth be told, I would've _asked_ about being allowed to change the signature before changing it, but I would expect the interviewer to explicitly state that the signature needs to be retained if that is going to be used to invalidate an otherwise valid answer. – Flater Sep 22 '20 at 10:03
  • 1
    @CarlLeth: You also have to allow for the context of an interview question here. Based on the setup of the question, developing submethods doesn't quite seem like the target scope of the question at hand. Doesn't mean it can't be done, but both examiner and examinee should be aware that formulated questions should explicitly specify any specific end goal requirement. – Flater Sep 22 '20 at 10:04
18

The solution validity is defined by the requirements.

The solution 1 doesn’t comply with the non-functional requirement “do not change the signature”. This has nothing to do with recursion but with your interview conditions.

This being said, and from an SE point of view, both algorithms are not equivalent:

  • solution 2 is a tail recursion, which can easily be optimized as a loop by the compiler. It’s moreover a true recursion, that fully solves the problem based on a smaller instance of the same problem.
  • solution 1 (edited, see comments) is also tail recursive when closely checking the applicable rules. But it is in reality the solution to a different problem: it’s no longer “is it a palindrome” but “is it a palindrome starting at index ...”. It is certainly a clever adaptation of an iterative solution making it recursive with the iterator as an argument. The trick of the default parameter helps to stay close to the initial requirements. But not only does it not comply with the requirements, but in addition the function could be called with an explicit index beyond the half length or even beyond the bounds, which might produce wrong results.
Christophe
  • 74,672
  • 10
  • 115
  • 187
  • 4
    While I admire the skill of spotting tail recursion, keep in mind that in many development stacks it's just an academic exercise. [Not everybody supports it](https://stackoverflow.com/questions/37224520/are-functions-in-javascript-tail-call-optimized). – candied_orange Sep 02 '20 at 06:45
  • 4
    Solution 1 is also tail recursive and does not use backtracking. – Theodore Norvell Sep 02 '20 at 16:05
  • @TheodoreNorvell - Solution 1 is not tail recursive as written. It can easily be rewritten to be tail recursive. – David Hammen Sep 03 '20 at 06:16
  • 2
    @Christophe - There is no backtracking in solution 1. There's a big difference between non-tail recursive algorithms and backtracking algorithms. Moreover some languages / some implementations do not optimize for tail recursion. As of January 1, 2020 Safari was the only browser that supports tail call optimization for javascript. – David Hammen Sep 03 '20 at 06:25
  • 5
    Why do you say one is tail recursive and the other is not? Both end with `if (condition) return isPalindrome(...); else return false;`. There doesn't seem to be any structural difference there. But, even if there were this difference, you'd have a hard time convincing me that the tail-recursive solution which involves creating a copy of the string at every step would be more efficient than just passing around the index. – Bernhard Barker Sep 03 '20 at 07:55
  • I agree that the algorithms are not equivalent, but your argument is wrong. The correct argument is that solution 2 is abysmally inefficient because it needs to create so many string slice objects. – cmaster - reinstate monica Sep 03 '20 at 10:48
  • @DavidHammen Indeed, when I first wrote this answer, I looked at solution 1 a little bit too quickly on my phone, and was mislead to think it would recurse before the conditional. So there is indeed no backtracking. I’ll edit later today. – Christophe Sep 03 '20 at 12:27
  • @cmaster-reinstatemonica Indeed I was mislead when I looked too quickly at solution 1. I’ll edit later today. Solution 1 is a recursive implementation of an iterative algorithm. It has moreover the weakness that it can be called with parameters that might cause it to loop forever or trigger an exception, which -even if not explicitly mentioned - are not expected in view of the requirements ;-) – Christophe Sep 03 '20 at 12:38
  • 3
    @DavidHammen The only recursive call in solution 1 is `return isPalindrome(...)`. It's a recursive call followed immediately by a return. So, yes, it is tail recursive as written. – Theodore Norvell Sep 03 '20 at 15:22
  • @TheodoreNorvell After careful analysis, it appears that you are right. While for an optimizing compiler with statically typed languages such as C++ or Java, there would be no doubt about the possible tail call optimization for both solutions, I assumed that in JS, the existence of a variable used after the return statement could defeat the TCO identification (solution 2 does not have this case). But I found an article with a step by step analysis procedures for JS code according to the JS specs that confirms your statement. Thank you for your insistance, I’ve edited accordingly. – Christophe Sep 04 '20 at 07:11
  • @BernhardBarker There is a difference between the two cases: the solution 1 uses a local object after the recursive return. For a statically typed compiled language this would be clearly optimized away at compile time and tail call optimized. I assumed that dynamic JS could not do that and would have to take a more prudent approach. But meanwhile I found an article that analyses in detail how to check that TCO can be applied according to the JS specs, and it turns out that both solutions pass this check. I’ve edited accordingly. – Christophe Sep 04 '20 at 07:19
7

I see three aspects:

  1. Was the extra-argument answer correct?

I feel that would depend on how the question was asked. Were you asked to implement the given function signature, or just to check palindromes using recursion?

While being technically correct is the best kind of correct, it doesn't mean they'll be impressed.

  1. How to handle the interview situation?

The interviewer may insist on a given signature for different reasons, for example:

  • it was a part of the intended question that they forgot to state, or you didn't hear
  • they want to see if you can find another solution
  • they don't care about performance
  • they don't see the performance advantage of the index version

The first three seem quite likely to me: if they wanted the fastest and easiest way to detecet palindromes, they wouldn't impose restrictions like using recursion.

As I see it, you have two options: do it their way, or convince them of your way.

Convincing them seems risky, as it could only work if the reason they disagree is because missed the performance advantage, you explain it clearly, and their ego doesn't get hurt. You'll have to read the situation.

Solving it their way is less impressive, but safer. Probably the best way to get the job.

  1. Is the solution with two arguments good, generally?

Outside this interview context, I would say its about performance vs readability. Adding the index might be more performant, and I would probably prefer it for that reason. But the single-argument version is more readable to me, and would be preferred in languages that have string slices.

Mark
  • 662
  • 5
  • 12
4

I would personally give a problem where recursion is a more natural fit, but if this is what I had to work with, I would prefer solution 2.

The reason is that using an index is relatively rare in recursive algorithms in the wild. They usually overcomplicate things and make the state more difficult to reason about. It's a sign that you first thought of how you would solve this with an imperative loop, then converted it to recursion, rather than thinking about what the subproblem is.

It's also more difficult to tell what your base cases are. Does solution 1 handle an empty string? I can't tell at a glance. The point of exercises like this isn't efficiency, it's clarity. How much effort is it for a reader to tell if it's correct?

Karl Bielefeldt
  • 146,727
  • 38
  • 279
  • 479
  • 1
    I don't agree that using an index is uncommon. I use that technique frequently. It is pretty much mandatory with text or array processing if you want to avoid creating a lot of object copies. – rghome Sep 04 '20 at 06:00
  • 1
    Admittedly, favoring performance over clarity is more common than I would prefer. – Karl Bielefeldt Sep 04 '20 at 15:13
  • copies are only made in languages that don't support slices – Jasen Sep 05 '20 at 06:34
0

There are plenty of situations where there is a function that needs implementing, and a simple implementation uses a recursive function with an additional parameter. For example Quicksort, where the original function has one argument (an array, assuming it is possible to determine the number of array elements), and then you call a recursive function with the index of the first and last element of a sub array as arguments. That recursive function is likely invisible to the original caller.

gnasher729
  • 42,090
  • 4
  • 59
  • 119
0

I assume that the question was asked to see if you can apply functional reasoning. Technically, both solutions are recursive.

Solution 1 looks a lot like an iterative solution. The next “iteration” is done by calling the function (recursively) with an incremented index.

Solution 2 shows functional reasoning. It is the commonly accepted way to do recursion. Usually, proper recursion can have additional parameters that carry intermediate states. It is, however, highly uncommon to add a counter as parameter.

To the interviewer you insisting that solution 1 is an elegant solution might show (whether true or not) that you have a narrow set of tools for problem solving. Asking for recursion gives you the possibility to show that you know some functional way to solve problems. You showing an iterative solution might come off as ignorant to the power and elegance recursive functions might provide in contrast to loops.

There is a saying in programming: “If the only tool you have is a hammer, everything looks like a nail.” You could have showed that you also have a screwdriver ;-)

Simon
  • 1