Skip to content
This repository has been archived by the owner on Apr 26, 2022. It is now read-only.

Refactored object.find in before_filters #94

Merged
merged 1 commit into from May 20, 2013

Conversation

dragosmiron
Copy link
Contributor

No description provided.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 4c87d3c on dragosmiron:filter-refactorings into 9d43a2c on mezis:master.

@mezis
Copy link
Owner

mezis commented May 20, 2013

While I appreciate the effort, it's a pattern used in HT's application I positively dislike.
IMO state variables should only ever be used in a single method, or at least in very close methods.

With you new code, I read the code for a #create method for instance, and my brain is like

  • wait, where does this @user come from?
  • oh wait, might be a before_filter ... scroll up the file
  • ok here it is ... scroll down the file.

Yes, it's less DRY, but it's much less readable, too!

What do you think?

@dragosmiron
Copy link
Contributor Author

I just followed the convention that you have in the other controllers where this happens. This means that it's either-or, i just thought it would look nicer if all are the same. It's up to you off course

@dragosmiron
Copy link
Contributor Author

regarding your scrolling concern, I got quite lost in all the modules that are everywhere :)

@mezis
Copy link
Owner

mezis commented May 20, 2013

Fair enough; internal consistency trumps my above comment. Thanks @dragosmiron, merging!

@mezis
Copy link
Owner

mezis commented May 20, 2013

Also, which module got you lost?

mezis added a commit that referenced this pull request May 20, 2013
Refactored object.find in before_filters
@mezis mezis merged commit c905053 into mezis:master May 20, 2013
@dragosmiron
Copy link
Contributor Author

such as Idea::History::Comment::IsSubject, Notification::Base::CanBeSubject, etc
For me it looks like bunny hopping from one file to the other.
My main approach in this would at least be to put them in some files, cause at some points i keep forgetting where exactly they are written.
At least good old RubyMine finds them :)

@mezis
Copy link
Owner

mezis commented May 21, 2013

For me it looks like bunny hopping from one file to the other.

For me it looks like separation of concerns 😄

My main approach in this would at least be to put them in some files

Specific example? I'm not too sure what you mean.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants