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

refactor createChannel to use return error pattern #17

Merged
merged 2 commits into from
Nov 17, 2016

Conversation

brettallred
Copy link
Owner

@brettallred brettallred commented Nov 16, 2016

We were breaking the error handling pattern in go. This is a very small change fixes the pattern for the createChannel method.

if conn == nil {
log.Printf("Failed to open a channel: no connection")
return nil
return nil, nil //TODO Remove this guard clause
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not to return nil, errors.New("Failed to open a channel: no connection")?

Copy link
Owner Author

Choose a reason for hiding this comment

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

I remove the guard clause in a different PR

Copy link
Owner Author

Choose a reason for hiding this comment

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

I did it like this because I was making some videos on refactoring.

@zenovich
Copy link
Contributor

+2

* remove nil checks

* Refactor out Flag Arguments (#21)

* Remove concept of autoclosing channel

* Extract Method Refactor - Showing Composed Method Pattern (#22)

* Simple extract method refactor

* improved refactor

* change AutoClosingChannel to ConnectionClosingChannel

* Ba/errors video5 (#24)

* Refactor out the consume essages function

* Refactor create Queue, remove guard clause and unecessary code

* continue refactoring out methods
@brettallred brettallred merged commit cd9b4d5 into master Nov 17, 2016
@brettallred brettallred deleted the ba/errors_video branch November 17, 2016 21:54
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