Freecodecamp: [beta] QA of Object Oriented Programming Section

[no-stack-dub-sack]

This was originally opened for one specific challenge, but noticing a couple of other things along the way in this section so just going to consolidate them in this one issue (a comment for each). @HKuz, if there is perhaps already another issue for this, I'm guessing you'd be the one to know, I did a quick search but could not find anything.

Overall - I think these challenges are great! _Definitely_ a _major_ improvement over the existing OOP section!!!! Well done to whoever created them!!

General Comments / Issues:

• It seems that in this section, once you've solved a challenge, you can change or even erase the code, and the challenge will still pass. I've created a separate issue for this: #13021
• It would be nice to get outputs logging to the embedded challenge console so people can see the results of their code. We're often calling methods that log things to the console in these challenges, but they are not designed to log in the in-page console.

[no-stack-dub-sack]

Use Dot Notation to Access the Properties of an Object

(RESOLVED IN STAGING) ✅

This challenge only accepts the following as a solution:

console.log(dog.name);
console.log(dog.numLegs);

However, we do not explicitly state that 2 separate statements should be used, and the following does not pass:

console.log(dog.name, dog.numLegs);

I'm thinking we should either specify that 2 separate console.log() statements need to be made, or refactor the test to accept both solutions - I think I would prefer the latter option. Thoughts?

[no-stack-dub-sack]

Verify an Object's Constructor with instanceof:

(RESOLVED IN STAGING) ✅

• For some reason, the linter throws the warning Expected an assignment of function call and instead saw an expression when the correct solution myHouse instanceof House; is written. The challenge does pass though.
• Also, the seed code itself has a linter warning on load for a missing semi-colon.
• Lastly on this one, not sure if it's intentional, but it might be a little confusing - in previous challenges in this section, constructors are defined like this:

function House(numBedrooms) {
  this.numBedrooms = numBedrooms;
}

but in this challenge they switch to this syntax:

let House = function(numBedrooms) {
  this.numBedrooms = numBedrooms;
}

This isn't so much an issue as a possible source of confusion. If we do use both ways, I think that we should specifically note the difference, otherwise, just keep the syntax consistent throughout the section. Probably introducing both is a good idea though I think.

[no-stack-dub-sack]

Understanding Own Properties:

Just an observation on this one, and curious to see what other opinions are - but this challenge seems to revolve a bit more around for...in rather than hasOwnProperty, and for...in is not adequately explained in this section as of yet.

If we are assuming people have worked through the rest of the curriculum, I think this is OK, because this is at least covered in the Basic Data Structures section, but if we intend the sections to be independent of one another and not require prerequisites than we may want to revisit this? Not sure... just a thought.

[no-stack-dub-sack]

Understand the Constructor Property:

(RESOLVED IN STAGING) ✅

This challenge will pass with either solution:

function joinDogFraternity(candidate) {
  if (candidate instanceof Dog) {
    return true;
  }
  return false;
}

// OR:

function joinDogFraternity(candidate) {
  if (candidate.constructor === Dog) {
    return true;
  }
  return false;
}

I don't see this as being a major problem since instinct dictates that people will try the solution suggested to them first, however, I can also see this being an issue being opened down the line when a savvy camper tries both ways and wants to point this out.

We could just add a test case that says "message: 'your solution should use the constructor property'" and verify with a regex.

[no-stack-dub-sack]

Use Inheritance So You Don't Repeat Yourself:

This challenge I think is a bit confusing. The title refers to Inheritance, however, inheritance is never mentioned in the challenge - I realizes it ties in to the next challenge, so campers will quickly find out, but the way its presented is still a bit off-putting. Also, at the end of the challenge, we've made an Animal supertype, but we're left a little confused, because Animal, at this point, does not tie in to Cat and Dog. To quell the confusion a bit, I think we could make a slight change:

This phrase comes from the next challenge:

This and the next challenge will cover how to reuse Animal's methods inside Bird and Dog without defining them again. It uses a technique called inheritance.

Perhaps, to tie things in, a higher level overview similar to this, can instead be given in this challenge that ties all 3 together so it's understood they are a sequence, and actually introduces the concept that the challenge is titled after, and makes it clear that at the end of this challenge we are not yet done.

[no-stack-dub-sack]

Inherit Behaviors from a Supertype:

(RESOLVED IN STAGING) ✅

Super minor issue here - part of the seed for this reads as follows:

// Add your code below this line

let duck
let beagle

duck.eat(); // Should print "nom nom nom"
beagle.eat(); // Should print "nom nom nom"

This throws a linter error. I would propose the following instead:

Super minor issue here - part of the seed for this reads as follows:

let duck; // change this line
let beagle; // change this line

duck.eat(); // Should print "nom nom nom"
beagle.eat(); // Should print "nom nom nom"

[HKuz]

@no-stack-dub-sack - these are all great points, and I haven't seen any issues for this section although I was offline a chunk of yesterday. Thanks for going through this section in such detail 👍 Here are my thoughts (tl; dr - I agree with everything you brought up):

• Use Dot Notation to Access the Properties of an Object: tests should pass if someone uses one console.log statement.
• Verify an Object's Constructor with instanceof: we should fix the missing semi-colon and be consistent about the way House is defined. While there are multiple ways of doing things in JS, no need to confuse people learning this for the first time.
• Understanding Own Properties: On your point about the for...in - we generally took the view that it's okay to use concepts already covered in the curriculum. Campers can jump around as they want, but there is a flow to the topics. (Otherwise, challenges can get long/repetitive if they have to re-cover things before getting to the point). That said, I think it may be helpful to put in a note right under the example that briefly explains the syntax ("Recall that for...in does ...)
• Understand the Constructor Property: agree on your point to add using the constructor in the instructions
• Use Inheritance So You Don't Repeat Yourself: yup, good points to tie the challenges together better
• Inherit Behaviors from a Supertype: we should definitely add the semi-colons, and the comments are helpful, too

Let me know if you want to work on these - I'm jumping into another section (could work on this in a day or two), or we could open this up as help wanted 😄

[no-stack-dub-sack]

@HKuz Cool, thanks! I'm going to finish up the section, add a few more comments if I have any, and then we can decide at that point, but I'm thinking opening it up to Help Wanted will prob be best. Will keep you posted.

[no-stack-dub-sack]

Add Methods After Inheritance:

This challenge is slightly confusing because of the following test case:

Dog should have the bark() method as an own property.

The solution for this part is looking for this:

Dog.prototype.bark = function() {
    console.log('Woof!');
};

and while Dog.prototype.hasOwnProperty('bark') does return true (so this is, of course, correct), the source of the confusion comes from here (which is from Iterate Over All Properties):

With the information campers are given up to this point, they would likely assume that to make that test pass, they would have to define the bark method, directly on the object instance of Dog rather than on the prototype.

The distinction is that for instances of Dog, bark would _not_ be an own property, but that it _is_ an own property of Dog.prototype. So, this is a little confusing for someone just getting introduced to these concepts.

The simplest solution would be to change the test case to say:

Dog.prototype should have the bark() method as an own property.

Although, maybe a quick explanation is in order to let campers know that a prototype property of a prototype is actually an own property of that prototype? Wow, that's a tongue twister, so yeah, its a bit confusing, and I'm not sure what the best way to allay that confusion is...

[no-stack-dub-sack]

Use Closure to Protect Properties Within an Object from Being Modified Externally:

(RESOLVED IN STAGING) ✅

Small typo:

I think supposed to be "...outside of bird's definition." ?

[no-stack-dub-sack]

Understand the Immediately Invoked Function Expression (IIFE):

No major problems with this challenge - just a suggestion - while I realize anonymous IIFE's are the more common pattern (and the pattern that's used in the next challenge), perhaps here is a good place to mention that IIFE's can also be named, and that this might even be considered better practice (although less commonly seen) because it can make debugging easier since errors will be harder to track down if there are many anonymous IIFEs.

Thoughts?

[no-stack-dub-sack]

Use an IIFE to Create a Module:

I'd like to get some thoughts on this one... maybe @dhcodes or @Greenheart?
My issue is that I don't think the challenge does an adequate job of explaining why an IIFE makes sense in this scenario.

The solution calls for:

let funModule = (function() {
  return {
    isCuteMixin: function(obj) {
      obj.isCute = function() {
        return true;
      };
    },  
    singMixin: function(obj) {
      obj.sing = function() {
        console.log("Singing to an awesome tune");
      };
    }
  };
})();

so you can do something like:

function Bird () { }
let duck = new Bird();
funModule.singMixin(duck);
duck.sing();

however, you could achieve the same thing in a less verbose way, and without invoking a function at all, simply by just defining the object the IIFE returns.

The challenge says this:

The advantage of the module pattern is that all of the motion behaviors can be packaged into a single object that can then be used by other parts of your code.

But since this does not require an IIFE at all, I guess I would question the idea of introducing the concept here, or finding a stronger way to tie it in. This might be confusing / misleading because campers might think they NEED to do this to achieve this pattern when that's not the case.

Any thoughts?

[Greenheart]

@no-stack-dub-sack I agree that this isn't the best example of an IIFE. Would it be better if we provide a module as an example?

I think the core value of the IIFE is that you can create private properties and methods of your objects. It is really useful for decreasing the ways others can (mis)use your software in the hope that it makes things more reliable.

For example, you might have a small utility module for your vanilla js application where you only want to expose a few public methods since the rest are about to be changed/removed and would cause trouble if they got used in other parts of the codebase.

This site has a lot of good examples: https://toddmotto.com/mastering-the-module-pattern/

[no-stack-dub-sack]

@Greenheart Well in the last of the 2 IIFE challenges here, it is introduced as a module pattern, and I don't have an issue with that, I just think it could be more clearly explained _why_ use an IIFE, and that an IIFE does not necessarily have to be present to achieve the same functionality. I think this says it all:

core value of the IIFE is that you can create private properties and methods of your objects. It is really useful for decreasing the ways others can (mis)use your software in the hope that it makes things more reliable.

If we can just explain this, and let the user know "you can achieve the same functionality without an IIFE, but with an IIFE is a better way, and here's why...".

The current instructions say:

An immediately invoked function expression (IIFE) is often used to group related functionality into a single object or module.

To this let's just add: "While the same functionality can be achieved without an IIFE, the core value of the IIFE, in this context, is that you can create private properties and methods of your objects. This can be very useful for decreasing the ways others can (mis)use your software and make things much more reliable."

[Greenheart]

@no-stack-dub-sack This! :point_up:

I took the freedom to compile it and make some minor changes. Something like this is what we need :blush:
An <dfn>immediately invoked function expression</dfn> (IIFE) is often used to group related functionality into a single object or module. While the same functionality can be achieved without an IIFE, its core value in this context is that you can create private properties and methods for your objects. This can be very useful for decreasing the ways others can (mis)use your software, and make things much more reliable.

Possibly use <dfn> if the term hasn't been used in previous challenges.

[Greenheart]

I'll go ahead and update the tests for Use Dot Notation to Access the Properties of an Object!

[Greenheart]

Continuing with https://github.com/freeCodeCamp/freeCodeCamp/issues/12966#issuecomment-275974706.

The first problem is because the linter doesn't want us to write an expression that essentially is dead code (since we neither call a function or create a variable). To fix this, I suggest we assign the result to a variable.

The second and third problem could be fixed by changing the seed to use the code you proposed. Assigning function objects to variables can be taught either somewhere else, or by experience. I think we should be consistent with function X () {}

I'll fix this :smile:

[Greenheart]

Fixing https://github.com/freeCodeCamp/freeCodeCamp/issues/12966#issuecomment-276268534 :smile:

[Greenheart]

Fixing https://github.com/freeCodeCamp/freeCodeCamp/issues/12966#issuecomment-276565338

[Greenheart]

I'll fix https://github.com/freeCodeCamp/freeCodeCamp/issues/12966#issuecomment-276245864

[Greenheart]

I noticed that no challenge have solutions, so I'm currently working on a PR.

[Greenheart]

Understanding Own Properties:

(RESOLVED IN STAGING) ✅

The tests currently allows use of the built in method Object.keys(), but I think campers get better practice by using for...in combined with Object.hasOwnProperty().

[Greenheart]

Object Oriented Programming: Iterate Over All Properties:

(RESOLVED IN STAGING) ✅

Just discovered that this challenge has the same problem - it allows use of Object.keys() As I still don't think this should be allowed in these challenges, I'll create a PR adding the test case from this line

function Dog(name) {
  this.name = name;
}

Dog.prototype.numLegs = 4;

let beagle = new Dog("Snoopy");

let ownProps = Object.keys(beagle);
let prototypeProps = Object.keys(Dog.prototype);

Fixing this right away :smile:

[Greenheart]

Object Oriented Programming: Change the Prototype to a New Object

Insufficient instructions and tests. Should describe and eat just exist on the prototype, or should they be functions?

I think we should add tests to verify that they are functions.

[Greenheart]

Object Oriented Programming: Remember to Set the Constructor Property when Changing the Prototype

PR submitted :white_check_mark:

This challenge should probably format "constructor property" as <code>constructor</code> property to increase readability and consistency. What do you think?

For instance, the test message uses this format.

[Greenheart]

A general suggestion is that we replace let statements with const to show best practices. This video by @mpj explains it well!

[Greenheart]

Object Oriented Programming: Reset an Inherited Constructor Property:

PR Submitted :white_check_mark:

Minor typo: supertype's should probably be supertype's

[Greenheart]

Object Oriented Programming: Use an IIFE to Create a Module:

PR submitted :white_check_mark:

Minor typo: "Here is a example using it:" should be changed to "Here is an example using it:"

[Greenheart]

I'm working on a PR for https://github.com/freeCodeCamp/freeCodeCamp/issues/12966#issuecomment-277447340

[Greenheart]

Another general suggestion: I think we need to change the examples so campers can't just copy the example and change 1-2 things to complete the challenge.

The examples should show the concept, but not use properties or methods that are used by the challenge itelf. This ways, I think people will learn more from each challenge.

[iansibindi]

Ditch Custom CSS for Bootstrap

On Sat, Feb 4, 2017 at 9:11 PM, Samuel Plumppu [e-mail]
wrote:

Another general suggestion: I think we need to change the examples so
campers can't just copy the example and change 1-2 things to complete the
challenge.

The examples should show the concept, but not use properties or methods
that are used by the challenge itelf. This ways, I think people will learn
more from each challenge.

—
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/12966#issuecomment-277463832,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AX9USApjW3rwocbHWe2yoFLV0RegbkCCks5rZL9SgaJpZM4LxApU
.

[Greenheart]

@iansibindi It looks like you're subscribed to all messages of this repository. To disable it, visit https://github.com/freecodecamp/freecodecamp and click "Unwatch" in the top right.

Sorry for the inconvenience!

[no-stack-dub-sack]

@Greenheart I'm finding it a bit hard to follow each of these even with the links - I guess I perhaps should have opened an additional issue for each one! I meant to add a checklist the original comments too.

I'll start doing an initial review of the PRs, but do you think you could add a comment to each of the original comments if they have a PR open for them so we can track which issues have been addressed?

[Greenheart]

@no-stack-dub-sack Haha I've got to admit I can't follow it myself either, I just kept posting! :smile:

I'll add a big "PR open" (with link to it) to the beginning of each comment that is WIP/fixed.

[no-stack-dub-sack]

@Greenheart Perfect! Thanks! I've reviewed the first several PRs

[Greenheart]

I'll fix https://github.com/freeCodeCamp/freeCodeCamp/issues/12966#issuecomment-277457211

[Greenheart]

I'll fix https://github.com/freeCodeCamp/freeCodeCamp/issues/12966#issuecomment-277459744

[Greenheart]

I'll fix https://github.com/freeCodeCamp/freeCodeCamp/issues/12966#issuecomment-277447340

[Greenheart]

@no-stack-dub-sack Alright, still a few things to do here but I resolved some of them today!

[Greenheart]

Oops, closed it again :laughing:

[no-stack-dub-sack]

@Greenheart Whoa! A throwback - is there anything left to do in this one? Some major improvements were made!

[Greenheart]

@no-stack-dub-sack I don't know - there are some things left to do according to the comments above, but they might have been resolved in other places?

[vikramnr]

I think its not fixed yet. Let me know if you think otherwise

[QuincyLarson]

Great work everyone. I'm closing this issue and we can reopen more specific issues as they crop up.