3 ms·

About coding the“FizzBuzz” interview question

2 points by micheleriva 7y ago

rvz 7y ago
Good article, but you forgot one more thing: Readability. Given that there are zero comments in the code, you are giving the impression of "The code is the documentation" attitude which forces the programmer to infer via the function signatures only, which compromises clean-code and maintainability. It is important to explain what these functions and one-liners do, as a different set of eyes might not easily understand what it is. If a candidate did this in a pair-programming session, I'm afraid that I would not accept this solution unless a reasonable amount of documentation in the function signatures like JSDoc is present.
gregjor 7y ago
At first reading I thought the author intended satire. The first two code examples meet the stated requirements, except for an off-by-one error in both: for (let i = 1; i < 100; i++) { ... } Requirement: Print the numbers from 1 to 100. Oops. The second example confusingly appends to the output variable -- except for the case of outputting the number -- although that variable gets set to empty on every iteration of the loop, and only one if/else block will get executed. Minus one for clarity. Then the author goes off the rails and keeps going. But there is still a big problem about maintainability. What if we want to add more control structures? Let’s say we want to print “Foo” when a number is multiple of 7 and “Bar” when it’s multiple of 11... Possible future requirements are not called "maintainability." The author gives a good example of YAGNI, though. Then after calling out this supposed problem the author does not give an example of how to address the possible proliferation of if/else conditions, even after calling it a "bad, bad, bad idea." Instead he shows an even less idiomatic and less clear switch construction that has exactly the same problem, with even more possibility of error due to leaving out a break by accident. The supposed maintainability problem -- what if we have to deal with 7 and 11 in the future -- never gets addressed. Then it gets worse. To avoid the for loop, which has one local variable, the author creates an array merely to iterate over it. Let's think about how that scales to doing FizzBuzz from one to ten million, since the author brought up scalability (although that wasn't an original requirement). But now… let’s make the things harder. How would you approach the same problem without using loops? The problem explicitly describes a loop from 1 to 100. But... That is a tricky question and will show the candidate deep knowledge of that particular programming language. I mean, there are just a few programming languages that doesn’t support loops, so we’re all pretty addicted to them! Sorry, map() is just a loop, a loop with function call overhead. This code has another off-by-one error, apparent if you just run the code in the browser. It assigns the local variable num to i + 1, since array indices begin at 0, but uses i in all of the calls to isMultiple() when it should use num. Running the code I get FizzBuzz as the very first output line. Note that the for loop solution didn't have this problem, but now the details of Javascript array indexes have surfaced into the code and become a new problem to think about. This bug -- using i when the parameter is named num -- carries into the final example, which throws an error right off. That tells me the programmer didn't test this code, or even read through it carefully. Looking at the test suite I see that it doesn't check for boundary conditions, like passing 0 to isMultiple(). I would want to discuss what isMultiple() should do when passed a 0. At this point the code bears almost no resemblance to the original problem statement, so as a hiring manager I would give a C- for a whiteboard exercise, and an F for a published article. I've seen several off-by-one and variable naming errors in the iterations, which looks careless, especially for code I would expect the programmer to write iteratively in a JS console. I've seen simple code balloon into something complex and unreadable, and I can't just copy/paste it into a Javascript console anymore. I wanted to see if the candidate can code a simple problem at all, not for a demonstration of increasingly obscure techniques and scope creep. This programmer may know Javascript, but they are going to drop spanners into every task I assign, and waste other team members' time polishing turds like this.
- gregjor 7y ago
  What I expect to see as an interviewer: for (let i = 1; i <= 100; i++) { let output = ""; // test modulos in ascending order if (i % 3 === 0) output += "Fizz"; if (i % 5 === 0) output += "Buzz"; if (output === "") output += i; console.log(output); } Look Ma, I can maintain it! for (let i = 1; i <= 100; i++) { let output = ""; // test modulos in ascending order if (i % 3 === 0) output += "Fizz"; if (i % 5 === 0) output += "Buzz"; if (i % 7 === 0) output += "Foo"; if (i % 11 === 0) output += "Bar"; if (output === "") output += i; console.log(output); } Since I didn't introduce any new functions like isMultiple as a unnecessary shorthand for the built-in modulo operator, and I didn't introduce unnecessary arrays to iterate over the index of, I don't need to write a test suite. I can simply compare the output to the expected results. Not having the function call for every value will help this "scale" if necessary, though for this problem scaling was neither stated as a requirement, nor implied by any reasonable reading of the requirements. Putting on my hiring manager hat, if a candidate changes a for loop to a map() because "there are just a few programming languages that doesn’t support loops," I would ask the candidate to name some of those languages, and how one could solve a problem like FizzBuzz without loops. I would expect to talk about recursion and declarative languages (Prolog, or even SQL), not about `map()`.

2 points by micheleriva 7y ago

rvz 7y ago

Good article, but you forgot one more thing: Readability. Given that there are zero comments in the code, you are giving the impression of "The code is the documentation" attitude which forces the programmer to infer via the function signatures only, which compromises clean-code and maintainability. It is important to explain what these functions and one-liners do, as a different set of eyes might not easily understand what it is. If a candidate did this in a pair-programming session, I'm afraid that I would not accept this solution unless a reasonable amount of documentation in the function signatures like JSDoc is present.

gregjor 7y ago

At first reading I thought the author intended satire. The first two code examples meet the stated requirements, except for an off-by-one error in both: for (let i = 1; i < 100; i++) { ... } Requirement: Print the numbers from 1 to 100. Oops. The second example confusingly appends to the output variable -- except for the case of outputting the number -- although that variable gets set to empty on every iteration of the loop, and only one if/else block will get executed. Minus one for clarity. Then the author goes off the rails and keeps going. But there is still a big problem about maintainability. What if we want to add more control structures? Let’s say we want to print “Foo” when a number is multiple of 7 and “Bar” when it’s multiple of 11... Possible future requirements are not called "maintainability." The author gives a good example of YAGNI, though. Then after calling out this supposed problem the author does not give an example of how to address the possible proliferation of if/else conditions, even after calling it a "bad, bad, bad idea." Instead he shows an even less idiomatic and less clear switch construction that has exactly the same problem, with even more possibility of error due to leaving out a break by accident. The supposed maintainability problem -- what if we have to deal with 7 and 11 in the future -- never gets addressed. Then it gets worse. To avoid the for loop, which has one local variable, the author creates an array merely to iterate over it. Let's think about how that scales to doing FizzBuzz from one to ten million, since the author brought up scalability (although that wasn't an original requirement). But now… let’s make the things harder. How would you approach the same problem without using loops? The problem explicitly describes a loop from 1 to 100. But... That is a tricky question and will show the candidate deep knowledge of that particular programming language. I mean, there are just a few programming languages that doesn’t support loops, so we’re all pretty addicted to them! Sorry, map() is just a loop, a loop with function call overhead. This code has another off-by-one error, apparent if you just run the code in the browser. It assigns the local variable num to i + 1, since array indices begin at 0, but uses i in all of the calls to isMultiple() when it should use num. Running the code I get FizzBuzz as the very first output line. Note that the for loop solution didn't have this problem, but now the details of Javascript array indexes have surfaced into the code and become a new problem to think about. This bug -- using i when the parameter is named num -- carries into the final example, which throws an error right off. That tells me the programmer didn't test this code, or even read through it carefully. Looking at the test suite I see that it doesn't check for boundary conditions, like passing 0 to isMultiple(). I would want to discuss what isMultiple() should do when passed a 0. At this point the code bears almost no resemblance to the original problem statement, so as a hiring manager I would give a C- for a whiteboard exercise, and an F for a published article. I've seen several off-by-one and variable naming errors in the iterations, which looks careless, especially for code I would expect the programmer to write iteratively in a JS console. I've seen simple code balloon into something complex and unreadable, and I can't just copy/paste it into a Javascript console anymore. I wanted to see if the candidate can code a simple problem at all, not for a demonstration of increasingly obscure techniques and scope creep. This programmer may know Javascript, but they are going to drop spanners into every task I assign, and waste other team members' time polishing turds like this.

gregjor 7y ago
What I expect to see as an interviewer: for (let i = 1; i <= 100; i++) { let output = ""; // test modulos in ascending order if (i % 3 === 0) output += "Fizz"; if (i % 5 === 0) output += "Buzz"; if (output === "") output += i; console.log(output); } Look Ma, I can maintain it! for (let i = 1; i <= 100; i++) { let output = ""; // test modulos in ascending order if (i % 3 === 0) output += "Fizz"; if (i % 5 === 0) output += "Buzz"; if (i % 7 === 0) output += "Foo"; if (i % 11 === 0) output += "Bar"; if (output === "") output += i; console.log(output); } Since I didn't introduce any new functions like isMultiple as a unnecessary shorthand for the built-in modulo operator, and I didn't introduce unnecessary arrays to iterate over the index of, I don't need to write a test suite. I can simply compare the output to the expected results. Not having the function call for every value will help this "scale" if necessary, though for this problem scaling was neither stated as a requirement, nor implied by any reasonable reading of the requirements. Putting on my hiring manager hat, if a candidate changes a for loop to a map() because "there are just a few programming languages that doesn’t support loops," I would ask the candidate to name some of those languages, and how one could solve a problem like FizzBuzz without loops. I would expect to talk about recursion and declarative languages (Prolog, or even SQL), not about `map()`.