Skip to content

Commit

Permalink
use self.method syntax to resolve circular argument issues
Browse files Browse the repository at this point in the history
  • Loading branch information
tmm1 committed Jan 3, 2015
1 parent 2f55808 commit 3a30b12
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,15 @@ def count_records
[options[:limit], count].compact.min
end

def has_cached_counter?(reflection = reflection())
def has_cached_counter?(reflection = self.reflection)
owner.attribute_present?(cached_counter_attribute_name(reflection))
end

def cached_counter_attribute_name(reflection = reflection())
def cached_counter_attribute_name(reflection = self.reflection)
"#{reflection.name}_count"
end

def update_counter(difference, reflection = reflection())
def update_counter(difference, reflection = self.reflection)
if has_cached_counter?(reflection)
counter = cached_counter_attribute_name(reflection)
owner.class.update_counters(owner.id, counter => difference)
Expand All @@ -77,7 +77,7 @@ def update_counter(difference, reflection = reflection())
# it will be decremented twice.
#
# Hence this method.
def inverse_updates_counter_cache?(reflection = reflection())
def inverse_updates_counter_cache?(reflection = self.reflection)
counter_name = cached_counter_attribute_name(reflection)
reflection.klass.reflect_on_all_associations(:belongs_to).any? { |inverse_reflection|
inverse_reflection.counter_cache_column == counter_name
Expand Down
2 changes: 1 addition & 1 deletion activesupport/lib/active_support/values/time_zone.rb
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ def at(secs)
#
# Time.zone.now # => Fri, 31 Dec 1999 14:00:00 HST -10:00
# Time.zone.parse('22:30:00') # => Fri, 31 Dec 1999 22:30:00 HST -10:00
def parse(str, now=now())
def parse(str, now=self.now)
parts = Date._parse(str, false)
return if parts.empty?

Expand Down

9 comments on commit 3a30b12

@parndt
Copy link
Contributor

@parndt parndt commented on 3a30b12 Mar 18, 2015

Choose a reason for hiding this comment

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

FYI, this never got applied to master

@rafaelfranca
Copy link
Member

Choose a reason for hiding this comment

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

Is it needed? Master is running fine with Ruby 2.2

@parndt
Copy link
Contributor

@parndt parndt commented on 3a30b12 Mar 18, 2015

Choose a reason for hiding this comment

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

Perhaps not, but I just noticed that the code there was still reflection = reflection() so I wanted to point it out to be sure!

@parndt
Copy link
Contributor

@parndt parndt commented on 3a30b12 Mar 18, 2015

Choose a reason for hiding this comment

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

Maybe in master we should just rename the variable in the method signature?

@rafaelfranca
Copy link
Member

@rafaelfranca rafaelfranca commented on 3a30b12 Mar 18, 2015 via email

Choose a reason for hiding this comment

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

@tmm1
Copy link
Contributor Author

@tmm1 tmm1 commented on 3a30b12 Mar 18, 2015

Choose a reason for hiding this comment

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

reflection = reflection() and reflection = self.reflection are equivalent, it's just a matter of style. There was some comments preferring the latter style so I switched when I rolled the second version of the 2.2 compat PR.

@parndt
Copy link
Contributor

@parndt parndt commented on 3a30b12 Mar 18, 2015

Choose a reason for hiding this comment

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

@tmm1 cool, thanks for that!

@parndt
Copy link
Contributor

@parndt parndt commented on 3a30b12 Mar 18, 2015

Choose a reason for hiding this comment

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

This is along the lines of what I was thinking: parndt@4cbe171

Should I bother opening a pull request? 😄

@arthurnn
Copy link
Member

Choose a reason for hiding this comment

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

@parndt Good that I came here, and saw this comments, was just about to push to master a similar change.
Wanna create a PR with your commit? also I would be more explicit and would do something like (_reflection = self.reflection)
thanks

Please sign in to comment.