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

Maybe overrides more parent methods #2437

Merged
merged 2 commits into from May 24, 2017

Conversation

telephon
Copy link
Member

This fixes #2436

A Maybe object lazily constructs an operation as an object instead of
performing it immediately. Its implementation remains incomplete
necessarily, in the current form. This commit adds a few common
collection methods and fixes the example in the helpfile.
@telephon telephon changed the title Topic maybe understands less Maybe overrides more parent methods Oct 28, 2016
^this.composeNAryOp(\myMethod, args)
}
}
::
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO extensions to built-in classes are a bad habit. I'm not necessarily saying that you should remove this example, but maybe warn of the possible dangers.

Copy link
Member Author

Choose a reason for hiding this comment

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

Extensions for existing classes are not bad – this was the point about adding class extensions in the first place, I think. What you mean, I suppose, is that here we make the subclass override methods of its superclass?

@nhthn nhthn added the comp: class library SC class library label Oct 28, 2016
@crucialfelix crucialfelix modified the milestone: 3.9 Nov 4, 2016
@telephon
Copy link
Member Author

I think this is fine. Opinions?

Copy link
Contributor

@nhthn nhthn left a comment

Choose a reason for hiding this comment

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

Looks okay to me, although I don't understand Maybe too well.

@telephon
Copy link
Member Author

ping

@nhthn
Copy link
Contributor

nhthn commented May 24, 2017

honk honk, anyone else want to look at this?

Copy link
Contributor

@mossheim mossheim left a comment

Choose a reason for hiding this comment

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

I think this looks good! Thanks for honking

@nhthn nhthn merged commit cd76d20 into supercollider:master May 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp: class library SC class library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants