fix(learn): update solutions in interview-prep challenges

[kmwarter]

Checklist:

• I have read freeCodeCamp's contribution guidelines.
• My pull request has a descriptive title (not a vague title like Update index.md)
• My pull request targets the master branch of freeCodeCamp.
• All the files I changed are in the same world language, for example: only English changes, or only Chinese changes, etc.

Closes #XXXXX

[gitpod-io]

[RandellDawson]

@kmwarter Overall, I think this is on the right track, but I have a couple of issues with a few of your changes.

1. You are using new Set as part of the solution. That is kind of cheating. I think the original idea was to create a Set data structure without using the built-in Set data type. I think a test should be added to validate this data type is not used for any of these Set challenges.
2. Even though they are not used in some of the challenges you made changes to, the values method you created does not return the same as the original method (in the challenge seed). Yours returns an array of property names and the original method returns an array of the property values (which is what it should be returning). I honestly think we could remove all the extra methods we are not specifically testing for a challenge. @ojeytonwilliams @moT01 What do you think about removing the methods we are not testing from the challenge seed. For the tests, we can always use the Before Test section to add any methods we need for testing (i.e. add).

[kmwarter]

@RandellDawson thanks for the feedback!

1. To this point, my intention here is not to use the built in Javascript Set data type as you mention, but to use my own Set. Since I define my Set in this scope this is the Set I refer to in my functions.

This can be checked with the following code:

class Set {
  constructor() {
    this.dictionary = {};
    this.length = 0;
  }
  randomMethodJsSetDoesntHave() {
    return true
  }
  union(set) {
    const newSet = new Set();
    console.log("IS THIS MY SET?", newSet.randomMethodJsSetDoesntHave())

    this.values().forEach(value => {
        newSet.add(value);
    })
    set.values().forEach(value => {
        newSet.add(value);
    })
    return newSet;
  }
}

2. I can fix this values() method and make sure it is consistent in all exercises and returning what it is supposed to. I also will wait for a response on this to see if I should remove all unrelated methods to the exercise at hand per your suggestion. I think that is a good idea. I think hiding them in the prototype if they are needed to do the tests is a better idea when needed.

[Manish-Giri]

This is a good solution. I wish there were some lessons upfront in the Coding Interview Prep section, to teach analysis of time and space complexity. With such concepts in mind, one can really understand how efficient an object/hashmap is, over simple arrays, for building data structures like Sets efficiently.

[ojeytonwilliams]

What do you think about removing the methods we are not testing from the challenge seed.

I think it can be helpful to see another implementation of code you've written, but it can be a little cluttered having unused functions lying around. How about a compromise? For the final challenge, show the all the methods, so the user can see the finished product (after they've completed that challenge). The rest would just have the methods they need.

if I should remove all unrelated methods to the exercise at hand per your suggestion. I think that is a good idea. I think hiding them in the prototype if they are needed to do the tests is a better idea when needed.

Agreed, that makes the most sense to me. Except for the final challenge, if we go that way.

[moT01]

I haven't looked through the other areas where there's unused methods but, in this particular group of challenges, all the methods are added by the user in previous challenges. So it's kind of like they are building this class/constructor function, one method/challenge at a time. So I'm kind of leaning towards keeping them in. I'm not against removing them, but I'm thinking I like it better with them left in.

Additionally, it would be nice if there's was a description for the purpose of this PR. The title is confusing and there's no description or issue to refer to. I'm not sure what this is trying to accomplish. And why are we changing all the solutions to class syntax, but leaving the seed code with the function syntax. I'm just a little confused.

[kmwarter]

@moT01 Great point about the challenge seed. I definitely think the solutions should be consistent in using the same basic structure as the seed (constructor vs. class). In fact, I would argue that one style should be used for all of the challenges to remain consistent. There should probably be one lesson in the beginning explaining the two ways but then the rest should either stick to constructor syntax or class syntax.

To your point about description. Good point also, I should have given a link to this other PR #38428
The purpose of this PR is to make all solutions consistent with the solution in 38428. 38428 gives students this solution that implements the Set correctly with a dictionary so that insertions, deletions, and has() methods are all O(1). The previous solution uses indexOf which makes this not really behave like a set and defeats the purpose of learning sets. Though I can see the original author's intent to likely simplify Sets for beginners. If this is going to be "Interview Prep" I don't think it should be targeting beginners, but regardless, anyone learning data structures needs to understand their importance, and the importance of Sets is not that they are like arrays but they only have unique values. The importance is that they are very useful in situations where you want an array like structure with only unique values because then you can store the unique values as keys and get the O(1) insertion, deletion, and has() methods.

So in conclusion it sounds to me like for this PR I should:

1. Keep the current structure where the solutions from previous exercises are kept to basically keep building the class out.
2. Fix these so the Seed starter uses class syntax consistent with the solution.
3. Be sure the value() method is correct across the board.

Lastly, a quick question while we are on this topic:
I think the Set lesson would make much more sense if it came after the Map lesson and maybe even mentioned that a Set should be implemented using a Map like structure as it is here?

I also spoke to Quincy recently and he said that there is a completely new curriculum site in the works. Does anyone know when/whether it matters to make these larger changes if this curriculum will be obsolete soon anyway?

[ojeytonwilliams]

Hi @kmwarter, after a bit of discussion amongst the mods, we basically agree with what you were saying. Just to make sure, could you replace the current seed code with your updated methods? Using the class syntax, of course.

I think the Set lesson would make much more sense if it came after the Map lesson and maybe even mentioned that a Set should be implemented using a Map like structure as it is here?

It can be moved, but I think that would be better in a separate PR.

Does anyone know when/whether it matters to make these larger changes if this curriculum will be obsolete soon anyway?

I think it's going to stick around.

[moT01]

The coding interview prep section is going to stay when the new curriculum is released as far as I know @kmwarter. So it's fine to continue to make improvements to it.

I think the Set lesson would make much more sense if it came after the Map lesson and maybe even mentioned that a Set should be implemented using a Map like structure as it is here?

For this PR, I would leave them in the current order to keep the PR focused. But certainly something we can discuss - maybe a new issue for that?

[kmwarter]

I have addressed all the comments we discussed in this latest PR. I will save moving this to after the Map structure for a future PR as you mention @moT01 .

I realized one thing in response to this comment by @RandellDawson though,
"the values method you created does not return the same as the original method (in the challenge seed). Yours returns an array of property names and the original method returns an array of the property values"

Since in this implementation the keys are the values Object.keys does return the same thing as the original values() method so I did not change this.

[RandellDawson]

@kmwarter Make sure to use 2 space indentation for the code.

[kmwarter]

I have fixed the indentation. Sorry about that! My eslint plugin does not seem to be working properly.

[moT01]

Are you satisfied with this @kmwarter?

[ojeytonwilliams]

Great work @kmwarter, this is much nicer!

I only found two things, both minor, let me know what you think.

[raisedadead]

Need to land this before merging the python curriculum

[moT01]

LGTM 🎉 - I added all these solutions to the guide already.