Freecodecamp: [beta] Bug in seed code for "Functional Programming: Refactor Global Variables Out of Functions"

Created on 5 Feb 2017  Â·  12Comments  Â·  Source: freeCodeCamp/freeCodeCamp

Challenge refactor-global-variables-out-of-functions has an issue.

According to the instructions, the goal of this challenge is to

Refactor (rewrite) the code so the global array bookList is not changed inside either function.

Another requirement (which doesn't seem to be be tested for) is that

Both functions should return an array

The problem is that the initial challengeSeed doesn't meet the second requirement since neither Array.push() nor Array.splice() returns the updated array. Since the challenge's intention seems to be to "Refactor Global Variables Out of Functions", I think this needs to be fixed, so campers can focus on removing global variables rather than making the code work.

To resolve these issues, I think we should

  • Make sure the functions in the challengeSeed returns the updated array, as their doc strings says.
  • Possibly add tests to verify that the functions do return an array.
// the global variable
var bookList = ["The Hound of the Baskervilles", "On The Electrodynamics of Moving Bodies", "Philosophiæ Naturalis Principia Mathematica", "Disquisitiones Arithmeticae"];

/* This function should add a book to the list and return the list */
// New parameters should come before the bookName one

// Add your code below this line
function add (bookName) {

  return bookList.push(bookName);

  // Add your code above this line
}

/* This function should remove a book from the list and return the list */
// New parameters should come before the bookName one

// Add your code below this line
function remove (bookName) {
  if (bookList.indexOf(bookName) >= 0) {

    return bookList.splice(0, 1, bookName);

    // Add your code above this line
    }
}

var newBookList = add(bookList, 'A Brief History of Time');
var newerBookList = remove(bookList, 'On The Electrodynamics of Moving Bodies');
var newestBookList = remove(add(bookList, 'A Brief History of Time'), 'On The Electrodynamics of Moving Bodies');

console.log(bookList);

help wanted learn

Most helpful comment

I really like the idea of only using local variables in functions, but it feels like this challenge is trying to do too much at one time.

Maybe we could split the concepts into two smaller challenges:

1. Refactor Global Variables out of Functions

  • Use similar challengeSeed to what we have now.
  • The goal is to replace the global variable bookList with a function-parameter (that campers decide the name for).

2. Avoid Mutating External Variables

  • Builds upon the challengeSeed from the previous challenge.
  • The goal is to teach why and how to avoid modifying variables you get passed to your functions.
  • In this case, this could be about using Array.prototype.slice() to create a copy before changing any data in the array with books.

What do you think? :blush:

All 12 comments

@Greenheart Can I solve this issue? If no one are working in this.

@fonsecapontes Sure! Refer to the contributing guidelines and come chat with us in /Contributors if you need help with anything :smile:

I really like the idea of only using local variables in functions, but it feels like this challenge is trying to do too much at one time.

Maybe we could split the concepts into two smaller challenges:

1. Refactor Global Variables out of Functions

  • Use similar challengeSeed to what we have now.
  • The goal is to replace the global variable bookList with a function-parameter (that campers decide the name for).

2. Avoid Mutating External Variables

  • Builds upon the challengeSeed from the previous challenge.
  • The goal is to teach why and how to avoid modifying variables you get passed to your functions.
  • In this case, this could be about using Array.prototype.slice() to create a copy before changing any data in the array with books.

What do you think? :blush:

Reconfirming the Bug.
Browser: Chrome 58.0.3029.81 (64 bit)

@Greenheart We just launched the new learning platform and expanded curriculum here: https://learn.freecodecamp.org - take a look at it and let me know what you think :)

Would you be interested in helping fix this challenge?

@QuincyLarson Please, someone fix this, it's been like that for sooooo long...
This challenge is really confusing, also splice method is used in wrong way, it doesn't return what it should.

any advance with this? i try 2 solutions using local variables inside off function and does't work, and when i try to use slice or indexOf method for arrays the console told me "this xxxx is not a function"

I've been working on fixing this and should have a pull request in by early
next week at the very latest.

On Wed, Oct 17, 2018, 17:25 Rabindranath Ferreira notifications@github.com
wrote:

any advance with this i try 2 solutions using local variables inside off
function and does't work, and when i try to use slice or indexOf method for
arrays the console told me "this xxxx is not a function"

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/freeCodeCamp/freeCodeCamp/issues/13157#issuecomment-430792340,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ARZO-P5c81LvOM0dh_MbGKTgTVgTIZiOks5ul6BWgaJpZM4L3gIY
.

Apologies for the multiple commits; I made a mistake and accidentally changed the commit hash a few times. Anything else going forward will be a commit to fix build issues.

this challenge is here now and seems to work. master seems to work locally.
The pr adds two tests checking array is returned and also works locally.

I just did this challenge and am inclined to agree - the current state of the challenge gets in the way of the lesson and confuses the message it's trying to convey.

@BurnhamG just wanted to point out that while your open pull request appears to fix the issue, there is another one.

@MilanAjtic pointed out above that:
return bookList.splice(0, 1, bookName);

Is very broken - it doesn't remove the book bookName at all, it blindly removes whatever the first book is and replaces it with bookName.

I'm wondering if this was an actual mistake in the original challenge, not an extra 'gotcha' left in on purpose?

@patricktownley Great catch! That was an actual mistake that I missed with past commits. I've made the appropriate changes in my pull request to take care of that issue as well.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

robwelan picture robwelan  Â·  3Comments

Tzahile picture Tzahile  Â·  3Comments

ROWn1ne picture ROWn1ne  Â·  3Comments

kokushozero picture kokushozero  Â·  3Comments

jurijuri picture jurijuri  Â·  3Comments