Skip to content

Proxy example fix #13

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gautam-ndk
Copy link

The proxy example was being terminated without completion because of the exception raised. Made minor changes.

@mentero
Copy link

mentero commented Feb 23, 2016

It's just an example to show how the pattern should be used. Comment explains that this code will raise error in that line so why catch it? It doesn't need to be runnable. You should be able to run it in your head not on your computer. I am downvoting this.

@gautam-ndk
Copy link
Author

Chill out man! Needed a runnable file without any exceptions for one of my app.

@mentero
Copy link

mentero commented Feb 23, 2016

And there is nothing wrong with that :) I just wanted to point out that I see little value (or even a slight loss) in accepting this PR. Please explain to me why you think it should be accepted if you think otherwise

@gautam-ndk
Copy link
Author

Hmm.. I think you misunderstood. I just asked you to chill out. I dint say there was something wrong. I was hoping someone else would look into this thread.

You see, it is fundamentally wrong when to have code after an exception raising statement. And in this case, we have the main logical block which shows the importance of proxy pattern after that exception raising statement, which I believe is wrong.

And I believe all the other samples are working code samples not just theoretical examples.

blah blah blah..blah... ..

No more ranting on this :-) Getting back to work!

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