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

Switch .count to .size where appropriate #1605

Merged
merged 17 commits into from
Sep 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/classes/pivotal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def get_stories(_verbose = false)
stories = []

request_stories("", stories)
request_stories("&offset=501", stories) if stories.count == 500
request_stories("&offset=501", stories) if stories.size == 500

stories
end
Expand Down
8 changes: 4 additions & 4 deletions app/classes/query/modules/high_level_queries.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ module Query::Modules::HighLevelQueries

# Number of results the query returns.
def num_results(_args = {})
@num_results ||= result_ids&.count || 0
@num_results ||= result_ids&.size || 0
end

# Array of all results, just ids.
Expand Down Expand Up @@ -67,7 +67,7 @@ def results(args = {})
# better all be valid instances of +model+ -- no error checking is done!!
def results=(list)
@result_ids = list.map(&:id)
@num_results = list.count
@num_results = list.size
@results = list.each_with_object({}) do |obj, map|
map[obj.id] ||= obj
end
Expand All @@ -78,7 +78,7 @@ def results=(list)
# better all be valid Integer ids -- no error checking is done!!
def result_ids=(list)
@result_ids = list
@num_results = list.count
@num_results = list.size
end

# Get index of a given record / id in the results.
Expand Down Expand Up @@ -112,7 +112,7 @@ def paginate_ids(paginator)
ids = ids.select { |id| @letters[id] == letter }
end
end
paginator.num_total = ids.count
paginator.num_total = ids.size
ids[paginator.from..paginator.to] || []
end
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pellaea is this OK in Query::HighLevelQueries? At the point of figuring out the paginator and the @num_results, the query is done, so it seems length is more appropriate.

It may make no difference currently, but it will probably matter if Query starts using scopes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok i now think size may be more appropriate.

Copy link
Member

Choose a reason for hiding this comment

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

Oh dear god! I can't possibly read that entire email thread right now!

I do want to make sure one thing is pointed out in this one case, though -- ids here is not a query, it's just a plain old Array. So there's no question of it executing SQL queries or anything if "done wrong".

(Is there in fact any difference between size, count and length on a plain old Array??... oh hold on, strike that question... it's almost certainly answered somewhere in the email thread I just deleted! :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

LOL


Expand Down
2 changes: 1 addition & 1 deletion app/classes/suggestion.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def name_with_most_observations(names)
best_name = nil
best_count = -1
names.each do |name|
count = name.observations.count
count = name.observations.size
next if count <= best_count

best_name = name
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/account/preferences_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ def update_pref(pref, val)
def update_content_filter(pref, val)
filter = ContentFilter.find(pref)
@user.content_filter[pref] =
if filter.type == :boolean && filter.prefs_vals.count == 1
if filter.type == :boolean && filter.prefs_vals.one?
val == "1" ? filter.prefs_vals.first : filter.off_val
else
val.to_s
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/collection_numbers_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ def figure_out_where_to_go_back_to
@back_object = Observation.safe_find(@back)
return if @back_object

@back_object = if @collection_number.observations.count == 1
@back_object = if @collection_number.observations.one?
@collection_number.observations.first
else
@collection_number
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/herbarium_records_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ def figure_out_where_to_go_back_to
@back_object = Observation.safe_find(@back)
return if @back_object

@back_object = if @herbarium_record.observations.count == 1
@back_object = if @herbarium_record.observations.one?
@herbarium_record.observations.first
else
@herbarium_record
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/names/synonyms_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ def split_off_desynonymized_names(main_name, checks)
pick_one = other_group.shift
pick_one.clear_synonym
other_group.each { |n| pick_one.transfer_synonym(n) }
main_name.clear_synonym if main_name.reload.synonyms.count <= 1
main_name.clear_synonym if main_name.reload.synonyms.size <= 1
end

def dump_sorter(sorter)
Expand Down
2 changes: 1 addition & 1 deletion app/helpers/locations_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ def country_link(country, count = nil)
# title of a link to Observations at a location, with observation count
# Observations at this Location(nn)
def show_obs_link_title_with_count(location)
"#{:show_location_observations.t} (#{location.observations.count})"
"#{:show_location_observations.t} (#{location.observations.size})"
end

def calc_counts(locations)
Expand Down
4 changes: 2 additions & 2 deletions app/helpers/object_link_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ def user_link(user, name = nil, args = {})
def user_list(title, users = [])
return safe_empty unless users&.any?

title = users.count > 1 ? title.to_s.pluralize.to_sym.t : title.t
title = users.size > 1 ? title.to_s.pluralize.to_sym.t : title.t
links = users.map { |u| user_link(u, u.legal_name) }
# interpolating would require inefficient #sanitize
# or dangerous #html_safe
Expand Down Expand Up @@ -163,7 +163,7 @@ def description_link(desc)
end

def observation_herbarium_record_link(obs)
count = obs.herbarium_records.count
count = obs.herbarium_records.size
if count.positive?

link_to((count == 1 ? :herbarium_record.t : :herbarium_records.t),
Expand Down
1 change: 1 addition & 0 deletions app/models/observation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -971,6 +971,7 @@ def dump_votes
end

# Has anyone proposed a given Name yet for this observation?
# Count is ok here because we have eager-loaded the namings.
def name_been_proposed?(name)
namings.count { |n| n.name == name }.positive?
end
Expand Down
2 changes: 1 addition & 1 deletion app/presenters/matrix_box_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ def user_to_presenter(user)
# user.contribution and observations.count are just numbers.
self.detail = "#{:list_users_joined.t}: #{user.created_at.web_date}<br/>
#{:list_users_contribution.t}: #{user.contribution}<br/>
#{:Observations.t}: #{user.observations.count}".html_safe
#{:Observations.t}: #{user.observations.size}".html_safe
# rubocop:enable Rails/OutputSafety
self.name = user.unique_text_name
self.what = user
Expand Down
4 changes: 2 additions & 2 deletions app/views/account/preferences/_filters.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@

<% ContentFilter.all.each do |filter| %>

<% if filter.type == :boolean && filter.prefs_vals.count == 1 %>
<% if filter.type == :boolean && filter.prefs_vals.one? %>
<%= render(partial: "account/preferences/filters/checkbox",
locals: { filter: filter, f: f }) %>
<% elsif filter.type == :boolean && filter.prefs_vals.count > 1 %>
<% elsif filter.type == :boolean && filter.prefs_vals.size > 1 %>
<%= render(partial: "account/preferences/filters/select",
locals: { filter: filter, f: f }) %>
<% elsif filter.type == [:string] %>
Expand Down
2 changes: 1 addition & 1 deletion app/views/collection_numbers/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ end
<%= form_with(model: @collection_number, url: url_params,
local: local, id: "collection_number_form") do |f| %>

<% if @collection_number.observations.count > 1 %>
<% if @collection_number.observations.size > 1 %>
<%= alert_block(
:warning, :edit_affects_multiple_observations.t(type: :collection_number)
) %>
Expand Down
2 changes: 1 addition & 1 deletion app/views/herbarium_records/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ end
<%= form_with(model: @herbarium_record, url: url_params,
local: local, id: "herbarium_record_form") do |f| %>

<% if @herbarium_record.observations.count > 1 %>
<% if @herbarium_record.observations.size > 1 %>
<%= alert_block(
:warning, :edit_affects_multiple_observations.t(type: :herbarium_record)
) %>
Expand Down
2 changes: 1 addition & 1 deletion app/views/names/synonyms/_fields_existing.html.erb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<%# existing synonyms %>

<% if current_synonyms.count > 1 %>
<% if current_synonyms.size > 1 %>
<%= fields_for(:existing_synonyms) do |fes| %>
<div class="form-group">
<%= content_tag(:div, "#{:form_synonyms_current_synonyms.t}:",
Expand Down
2 changes: 1 addition & 1 deletion app/views/observations/show/_collection_numbers.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ cn_query = Query.lookup(:CollectionNumber, :all, observations: obs.id)
</ul>

<% elsif numbers.any? && !can_edit %>
<%= numbers.count > 1 ? :Collection_numbers.t : :Collection_number.t %>:
<%= numbers.size > 1 ? :Collection_numbers.t : :Collection_number.t %>:
<%= numbers.map do |number|
link_to("<i>#{number.format_name.t}</i>".html_safe,
collection_number_path(id: number.id, q: cn_query),
Expand Down
2 changes: 1 addition & 1 deletion app/views/observations/show/_herbarium_records.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ query = Query.lookup(:HerbariumRecord, :all, observations: obs.id)

<% elsif records.any? && !can_add %>
<div>
<%= records.count > 1 ? :Herbarium_records.t : :Herbarium_record.t %>:
<%= records.size > 1 ? :Herbarium_records.t : :Herbarium_record.t %>:
</div>
<ul class="tight-list">
<% records.each do |record| %>
Expand Down
4 changes: 2 additions & 2 deletions app/views/pivotal/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@
%>

<div class="container-text">
<% if @stories.count > 900 %>
<b>Displaying <%= @stories.count %> of 1,000 available Feature Requests. Please <%= link_to 'let the admins know', '/emails/ask_webmaster_question', title: "Contact Us" %> that we're close to the limit.</b>
<% if @stories.size > 900 %>
<b>Displaying <%= @stories.size %> of 1,000 available Feature Requests. Please <%= link_to 'let the admins know', '/emails/ask_webmaster_question', title: "Contact Us" %> that we're close to the limit.</b>
<% end %>
<%= :pivotal_index_header.tp %>
</div><!--.container-text-->
Expand Down