Skip to content

Switched how methods should be written.#5

Closed
reissbaker wants to merge 1 commit intomasterfrom
dont_overwrite_prototype
Closed

Switched how methods should be written.#5
reissbaker wants to merge 1 commit intomasterfrom
dont_overwrite_prototype

Conversation

@reissbaker
Copy link
Contributor

Overwriting the prototype makes inheritance impossible: you'll overwrite the base class with your methods. I switched the recommended ways of writing methods so that we don't make it impossible to use inheritance.

@hshoff
Copy link
Member

hshoff commented Nov 2, 2012

Good save. Could you move the bad example above the good example? Just for consistency sake.

@ajacksified
Copy link
Contributor

Good point; but what about when initially composing the object, when you're defining a large amount on the prototype for the first time (assuming no inheritance)? Also, does that open up the doors to making monkeypatching acceptable?

@hshoff hshoff closed this Nov 2, 2012
@reissbaker
Copy link
Contributor Author

Re: defining the class for the first time: defining on the existing prototype still works just as well at class definition time as overwriting the prototype. Gzip does a great job at getting rid of those prototype references, too. Having a single way to define classes is nice for readability.

Re: monkeypatching: good god no. Adding a guide about that can be a separate pull request, though.

@hshoff hshoff reopened this Nov 2, 2012
@hshoff
Copy link
Member

hshoff commented Nov 2, 2012

Whoops, accidental close.

I'm with Matt on this. I'm scared of making assumptions about inheritance.

@hshoff
Copy link
Member

hshoff commented Nov 2, 2012

For real close this time. Fixed: @b2c7d3d

@hshoff hshoff closed this Nov 2, 2012
@hshoff hshoff deleted the dont_overwrite_prototype branch August 8, 2014 06:43
WithSoull added a commit to WithSoull/js-for-trpp that referenced this pull request Mar 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants