CODEWITHSUNDEEP
ruby-on-railsrubyrefactoring
Bhushan Lodha
Bhushan Lodha

Reputation: 6862

Refactoring following lines of code

- if clean_book_intro_component(:literary_works).present?
  %h2 Other Books Related to #{@book.title}
  %p.fact-text
    = clean_book_intro_component(:literary_works)

Can above code written by calling clean_book_intro_component(:literary_works) only once?

Implementation of clean_book_intro_component

def clean_book_intro_component(component)
  sanitize @book.intro.send(component), tags: %w(span b i p a), attributes: %w(id class style href)
end

Upvotes: 2

Views: 56

Answers (4)

Damiano Stoffie
Damiano Stoffie

Reputation: 897

- clean_book_intro_component(:literary_works).tap do |cbic|
  - if cbic.present?
    %h2 Other Books Related to #{@book.title}
    %p.fact-text
      = cbic

maybe I got haml wrong, but the idea is to use tap instead of explicitly saving the result of the call

Upvotes: 1

spickermann
spickermann

Reputation: 106872

You could move the view into a partial and call the partial on a collection. If the collection is empty nothing is rendered. Something like:

# a related_books partial
%h2 Other Books Related to #{@book.title}
  %p.fact-text
    = related_books


# in the calling view
= render partial: related_books, collection: [clean_book_intro_component(:literary_works)]

See the Rails Guide about rendering collections.

Upvotes: 0

Simone Carletti
Simone Carletti

Reputation: 176402

Assigning variables in view is generally not recommended. However, in this case you can use an inline assignment which is generally accepted (as long as you keep the usage scoped within the context of the condition block):

if (intro = clean_book_intro_component(:literary_works)).present?
  %h2 Other Books Related to #{@book.title}
  %p.fact-text
    = intro

Another solution is to memoize the value inside the function.

def clean_book_intro_component(component)
  @component ||= {}
  @component[component] ||= sanitize @book.intro.send(component), tags: %w(span b i p a), attributes: %w(id class style href)
end

However, this will cause the interpreter to retain the data as long as there is a reference to the instance. Therefore, this is recommended only in very particular cases where the execution is expensive and/or the data to be memoized is limited.

Moreover, it requires some extra complications if the helper accepts parameters. In fact, you will end up memoizing an amount of data which is linear with the possible input of the parameters.

Upvotes: 1

pablochan
pablochan

Reputation: 5715

Yes, just save the result of clean_book_intro_component(:literary_works) in a variable and then use that variable instead of invoking the function.

Upvotes: 0

Related Questions