Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Concept Exercise: Windowing System (Classes) #1476

Merged
merged 27 commits into from
Dec 28, 2021

Conversation

junedev
Copy link
Member

@junedev junedev commented Oct 30, 2021

Resolves #1015

@github-actions
Copy link
Contributor

github-actions bot commented Oct 30, 2021

Dear junedev

Thank you for contributing to the JavaScript track on Exercism! 💙
You will see some automated feedback below 🤖. It would be great if you can make sure your PR covers those points. This will save your reviewer some time and your change can be merged quicker.

  • 📜 The following files usually contain very similar content.

    • concepts/<concept>/about.md
    • concepts/<concept>/introduction.md
    • exercises/concept/<exercise>/.docs/introduction.md

    Please check whether the changes you made to one of these also need to be applied to the others.

  • 🧦 If you changed the function signature or the JSDoc comment in the exemplar file (.meta/exemplar.js) or the stub file (<exercise>.js), make sure the change is applied to both files.

  • ✍️ If your PR is not related to an existing issue (and is not self-explaining like a typo fix), please make sure the description explains why the change you made is necessary.

  • 🔤 If your PR fixes an easy to identify typo, if would be great if you could check for that typo in the whole repo. For example, if you found Unicdoe, use "replace all" in your editor (or command line magic) to fix it consistently.

Dear Reviewer/Maintainer

  • 📏 Make sure you set the appropriate x:size label for the PR. (This also works after merging, in case you forgot about it.)

  • 🔍 Don't be too nit-picky. If the PR is a clear improvement compared to the status quo, it should be approved as clear signal this is good to be merged even if the minor comments you might have are not addressed by the contributor. Further improvement ideas can be captured in issues (if important enough) and implemented via additional PRs.

  • 🤔 After reviewing the diff in the "Files changed" section, take a moment to think about whether there are changes missing from the diff. Does something need to be adjusted in other places so the code or content stays consistent?

Automated comment created by PR Commenter 🤖.

@junedev junedev force-pushed the add-concept-exercise-classes branch 2 times, most recently from 6d7132d to ba2a7e9 Compare October 31, 2021 14:00
@junedev
Copy link
Member Author

junedev commented Nov 2, 2021

@pertrai1 Please wait with the review until the PR is un-drafted/ready for review. I often rewrite the texts along the way.

I only push to Github to make the status transparent and as a backup of the work.

@junedev junedev linked an issue Dec 2, 2021 that may be closed by this pull request
@junedev junedev changed the title Add Concept Exercise: Elons Toys (Classes) Add Concept Exercise: Windowing System (Classes) Dec 2, 2021
@junedev
Copy link
Member Author

junedev commented Dec 3, 2021

@SleeplessByte The about.md is done now, it was tough 😅, resources were similarly bad/fragmented as with type conversion. It would be great if you could review it. Please focus on whether something is wrong or something extremely important is missing (I know there is also more we could cover at some point). Also let me know in case you have written any good articles on the topic that I should link. I am neither an expert nor a fan of OPP, so please also check whether the terminology seems ok.

@pertrai1 You can of course also review about.md now if you like.

Copy link
Member

@SleeplessByte SleeplessByte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking great. Few suggestions.

concepts/classes/.meta/config.json Outdated Show resolved Hide resolved
concepts/classes/about.md Outdated Show resolved Hide resolved
concepts/classes/about.md Show resolved Hide resolved
concepts/classes/about.md Outdated Show resolved Hide resolved
concepts/classes/about.md Show resolved Hide resolved
concepts/classes/about.md Show resolved Hide resolved
concepts/classes/about.md Show resolved Hide resolved
concepts/classes/about.md Show resolved Hide resolved
concepts/classes/about.md Outdated Show resolved Hide resolved
concepts/classes/about.md Outdated Show resolved Hide resolved
@junedev junedev marked this pull request as ready for review December 27, 2021 19:20
@junedev junedev requested a review from a team December 27, 2021 19:21
@junedev junedev added the x:size/large Large amount of work label Dec 27, 2021
@junedev
Copy link
Member Author

junedev commented Dec 27, 2021

@SleeplessByte Thanks again for the review of the concept content. I liked all the suggestions and applied the respective changes. I filled in the missing introduction files now that the about content was fixed so this is now ready to go once someone reviewed the rest.

That means we can add fields to the new instance by adding them to `this` in the constructor function.

```javascript
function Car(color, weight) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if it is helpful to show how this is constructed under the hood as an object:

function Car(color, weight) {
    let thisObj = Object.create(null);
    thisObj.color = color;
    thisObj.weight = weight;
    return thisObj;
}

this removes the need to explicitly set the object and return it. Might be overkill or not needed :-)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the student might not know about Object.create at this point, I felt this code is more confusing then helping. Imo the important part is that you don't have to explicitly return this and that is covered in the text above.

this is automatically returned from the constructor function ...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds great @junedev. This concept looks awesome and enjoyed reading the code when reviewing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(fun fact, returning a new object from the constructor function is in fact a way to get closures/private variables and private functions from a constructor, however, you need to set the prototype at Object.create if you want to keep inheritance to work)

@junedev junedev merged commit b65b981 into exercism:main Dec 28, 2021
@junedev junedev deleted the add-concept-exercise-classes branch December 28, 2021 16:51
@SleeplessByte
Copy link
Member

Let's simulate some windows 🌊🌪️!
In this exercise, you will be simulating a windowing based computer system. You will create some windows that can be moved and resized. Find your way around classes (and learn a bit about prototypes in the process)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
x:size/large Large amount of work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement new Concept Exercise: Classes
3 participants