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 SearchHelper #1004

Open
eoinkelly opened this issue May 23, 2020 · 0 comments
Open

Refactor SearchHelper #1004

eoinkelly opened this issue May 23, 2020 · 0 comments

Comments

@eoinkelly
Copy link
Contributor

SearchHelper makes extensive use of instance variables. This is a bit messy and Rubocp doesn't like it so it would be great to refactor these helpers to not depend on instance variables by turning them into pure functions which are passed all the args they need and don't mutate any of their args (i.e. they return copies).

List of disabled cops in app/helpers/search_helper.rb

[17:05:27] ❯ ag "rubocop:disable"
app/helpers/search_helper.rb
3:module SearchHelper # rubocop:disable Metrics/ModuleLength
44:    return if @query[:hs].blank? # rubocop:disable Rails/HelperInstanceVariable
46:    query_hs = @query[:hs] # rubocop:disable Rails/HelperInstanceVariable
49:    query_hs = @query[:hs].map { |q| "#{q}.1" } if shape.split('.').last == '1' # rubocop:disable Rails/HelperInstanceVariable
55:    'selected' if @query[:l].present? && @query[:l].include?(location.split('.')[1]) # rubocop:disable Rails/HelperInstanceVariable
59:    'selected' if @query[:lg].present? && @query[:lg].include?(location_group.split('.')[0]) # rubocop:disable Rails/HelperInstanceVariable
85:    keys = @query.select { |_key, value| value.present? }.keys # rubocop:disable Rails/HelperInstanceVariable
102:    return if @query[:l].blank? # rubocop:disable Rails/HelperInstanceVariable
120:    return if @query[:hs].blank? # rubocop:disable Rails/HelperInstanceVariable
137:    return if @query[:lg].blank? # rubocop:disable Rails/HelperInstanceVariable
153:    h SignMenu.usage_tags.select { |u| @query[:usage].include?(u.last.to_s) }.map(&:first).join(' ') if @query[:usage].present? # rubocop:disable Rails/HelperInstanceVariable
157:    h SignMenu.topic_tags.select { |u| @query[:tag].include?(u.last.to_s) }.map(&:first).join(' ') if @query[:tag].present? # rubocop:disable Rails/HelperInstanceVariable
161:    return if @query[key].blank? || (@query[key].is_a?(Array) && @query[key].reject(&:blank?).blank?) # rubocop:disable Rails/HelperInstanceVariable
163:    h @query[key].join(' ') # rubocop:disable Rails/HelperInstanceVariable
176:    @display_search_term ||= safe_join(search_term_elements, ' ') # rubocop:disable Rails/HelperInstanceVariable

app/helpers/application_helper.rb
5:    "#{@title}#{' -' if @title} #{t('layout.title')}" # rubocop:disable Rails/HelperInstanceVariable

app/helpers/vocab_sheet_helper.rb
5:    return nil if @sheet.blank? # rubocop:disable Rails/HelperInstanceVariable
6:    return nil if @sheet.items.length.zero? # rubocop:disable Rails/HelperInstanceVariable
13:    return offsets[:"size_#{@size}"] if @size.present? # rubocop:disable Rails/HelperInstanceVariable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant