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

Edits & Suggestions for Events Chapter #1

Merged
merged 21 commits into from
Jul 6, 2017

Conversation

mikewesthad
Copy link

@mikewesthad mikewesthad commented Jul 3, 2017

@roymacdonald, here's the PR to go along with the general feedback in openframeworks#256. I tried to break up the edits and suggestions by section (at least, as much as git's diff allowed...). There are a number of suggestion that I made inline using the format: MH: blah blah blah. Those were either bigger changes or questions I had.

@roymacdonald
Copy link
Owner

Hey mike. This is awesome. Many thanks.
I'll review this later today and let you know .
All the best!

@mikewesthad
Copy link
Author

@roymacdonald - no problem, happy to help. Take this PR and the general comments from openframeworks#256 as suggestions - cherry pick as you see fit.


If you comment out the line that says `#define STOP_EVENT_PROPAGATION`, the mouse events will not stop propagation.

ofApp.h
**MH: the preprocessor directives here make it harder to see the core point about event propagation. I can see why you wanted them, but I would remove them. Instead, you can have a comment somewhere that says "try commenting out the `return bDragging;` lines and see what happens when you don't stop the event propagation"**
Copy link
Owner

Choose a reason for hiding this comment

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

Its true. I wasn't sure which was the best way to handle this. The commenting/uncommenting approach could lead to errors or undesired behaviors in case that not all the things that need to be commented out are commented out.
The other solution I though of was to have two separate examples, one without events propagation, just as whats described at the beggining of this section, and another one with events propagation.
I think that this way there will be less cluttering stuff and make it easier to see the differences.

Copy link
Author

Choose a reason for hiding this comment

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

+1 to having two separate examples in your code folder. That seems like a nice solution because someone reading the chapter will be able to see the way to stop propagation clearly, but someone who wants to actually run the code would be able to see both.

In C++ there is a rule, called the rule of 3, which became the rule of 5 in C++11.
Read about it [here](https://en.wikipedia.org/wiki/Rule_of_three_(C%2B%2B_programming)).
In short it says that if you add to a class any of the following you must add them all.
In C++ there is a rule, called the rule of 3, which became the rule of 5 in C++11. Read about it [here](https://en.wikipedia.org/wiki/Rule_of_three_(C%2B%2B_programming)). In short, it says that if you one of the following methods to a class, you must add all of them:
Copy link
Owner

Choose a reason for hiding this comment

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

should say:
In short, it says that if you add one of the following methods to a class, you must add all of them:

Copy link
Author

Choose a reason for hiding this comment

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

Whoops - sorry about that

This new way for listening to events allows you to use lambda functions as the callback.
If you don't know about Lambda functions read ofBook's chapter about C++11. Even though when just a section of it is about Lambda functions read the whole chapter as it describes several super useful features.
Lambda functions are annonymous functions. This means that you dont have to declare these previously in order to use, you are able to assign these dynamically and pass as function argument.
This new way for listening to events allows you to use lambda functions as the callback. If you don't know about Lambda functions see the C++11 chapter. Lambda functions are anonymous functions. You can assign them dynamically and pass them as a parameter. You don't have to have to declare them before using them. It is possible to use a lambda function as the callback of any event.
Copy link
Owner

Choose a reason for hiding this comment

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

add link to the C++ chapter.

@@ -8,21 +8,12 @@ openFrameworks has a super powerful events system which you can use to write cle

The event system is based around the idea of broadcasting messages. A part of your code can send messages to other parts that are listening. This is called the ["observer pattern"](https://en.wikipedia.org/wiki/Observer_pattern). There are "subjects" (senders) and "observers" (listeners). Observers subscribe to the particular subject they are interested in listening to. When a subject decides to send a message, any listeners that are subscribed will be notified of that message.
Copy link
Owner

Choose a reason for hiding this comment

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

as for the rest of the chapter as well as for the function names I think that it is better to speak of senders and listeners. May just mention here that these are also referred to as subjects and observers, respectively.

@roymacdonald
Copy link
Owner

hey mike I will merge your PR. I'm super happy with your comments, review and insight.
There are some stuff I still will change from what you changed, but I think that it is easier and tidier to merge this PR and change on top of it instead of cherrypicking.

@roymacdonald roymacdonald merged commit fc6880c into roymacdonald:master Jul 6, 2017
@mikewesthad
Copy link
Author

Glad it was helpful! Looking forward to seeing the chapter go live.

Re: changes. Yeah, that makes sense. In retrospect, it would have been hard to break things down into a cherry-pickable set of commits.

@mikewesthad mikewesthad deleted the events-chapter-edits branch July 7, 2017 00:23
roymacdonald pushed a commit that referenced this pull request Dec 15, 2018
Fix colour vs color spelling.
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.

2 participants