Freecodecamp: Changing the value of your parameter is a code smell

Created on 4 Jul 2016  路  12Comments  路  Source: freeCodeCamp/freeCodeCamp

Challenge Factorialize a Number has an issue.
User Agent is: Mozilla/5.0 (Windows NT 6.3; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.103 Safari/537.36.
Please describe how to reproduce this issue, and include links to screenshots if possible.

I don't believe we should encourage beginners to change the value of a parameter of a method. In the proposed code, the function factorialize returns "num". It would be better form to create a var "result" to return it at the end - teaching good coding practices.

My code:

function factorialize(num) {
  var result = 1;
  for(var i=1;i<=num;i++) {
    result *= i;
  }
  return result;
}

factorialize(5);

Most helpful comment

I disagree that modifying input param is a bad practice, unless some one can point me to a concrete reference.

For instance, someone can rightly argue that each variable you declare is adding a new reference to the same object on the stack.

var actualVariable = 10;
var myVariable = actualVariable;
var yourVariable = myVariable;

All the above point to the location where 10 is stored, just with different references (variable names);

This is rather, waste of resources (not in this challenge), but when you do any memory intensive computing (visual renders, etc.)

Edit: Missed a point, that you could copy the data within a variable only when necessary, only when it is essential to work on a copy whilst preserving the original, else it is waste of assigning more variables than required.

IMHO modifying input param should be completely acceptable. JavaScript lets us do that and leveraging it should be the best practice not the other way round.

All 12 comments

@JonathanScher agreed. We can change the seed code to:

function factorialize(num) {
  var result;


  return result;
}

factorialize(5);

@erictleung I can take this. But is challengeSeed the only thing that needs to be modified? Or should solutions be modified as well?

@Manish-Giri go for it! And just the challengeSeed needs changing. The solution, which shows one very succinct solution, doesn't modify the input parameters.

@erictleung cool! Will do.

Would we not need to change this for every single instance? Like in Title Case a Sentence, Return Largest Numbers in Arrays, Confirm the Ending and so on...

@Bouncey mmm good point.

@FreeCodeCamp/issue-moderators what do you guys think about changing challenges so that it doesn't modify the input parameters?

I don't believe we should encourage beginners to change the value of a parameter of a method.

It is also perfectly acceptable to change that value.

The goal of the challenge is for the camper to develop an algorithm, how they do that is up to the user. At this point we are not trying to teach campers best practice. This comes up every once in a while and I understand why some would think that, but best practices is often more about opinion then clear cut fact.

We should concentrate on the substance of a challenge. Does this challenge teach something valuable? Yes. Does changing the value of num harm anything? No.

I disagree that modifying input param is a bad practice, unless some one can point me to a concrete reference.

For instance, someone can rightly argue that each variable you declare is adding a new reference to the same object on the stack.

var actualVariable = 10;
var myVariable = actualVariable;
var yourVariable = myVariable;

All the above point to the location where 10 is stored, just with different references (variable names);

This is rather, waste of resources (not in this challenge), but when you do any memory intensive computing (visual renders, etc.)

Edit: Missed a point, that you could copy the data within a variable only when necessary, only when it is essential to work on a copy whilst preserving the original, else it is waste of assigning more variables than required.

IMHO modifying input param should be completely acceptable. JavaScript lets us do that and leveraging it should be the best practice not the other way round.

While I agree with @BerkeleyTrue's point that the aim of the challenge is to help the camper to develop an algorithm and not necessarily talk about best practices in general, I don't think it's a good idea to modify method parameters. Here's an interesting article that I found on this context.

When I approved the change, I was thinking in a functional way, whereby you don't/can't modify input parameters 馃槃

However, thinking functionally is outside the scope of this challenge and the challenges that @Bouncey has listed. So I change my mind on having these challenges to not modify the input parameters and agree with @raisedadead and @BerkeleyTrue's stance on the matter.

Cool. This was really fun though, I might be biased from belonging to a C++ and native programming world (memory and execution speed is the most valuable resource for me), but on that note since all of us feel that we should NOT modify the input parameters, lets keep it that way! I can live with that :wink:

@Manish-Giri thanks for the share, its a good read!

@Manish-Giri Thanks for the article. It was an interesting read. It reminds me of an article I read when I first starting coding.

The arguments object is a tricky beast if you don't know what you are doing. But the author came to the wrong conclusion, best practice is to always typecast the arguments object to an array.

function() { var args = Array.prototype.slice.call(arguments); };

or with new and improved JavaScript es2015

function(...args) { };

This is common and indeed best practice when ever working with the arguments object. You can see that in the first couple of comments on that post.

Was this page helpful?
0 / 5 - 0 ratings