← Back to Upcase

Blending two sources - avoiding messy if/else


(Geoff Harcourt) #1

I’m having trouble writing a function without what looks like a messy if/else flow.

I have two sets of news stories (organization and parent), and I need to display ten in total. If both sources have at least five stories, I show five of each. If one source has fewer than five, I can take stories from the other source to make up the difference. Right now I have two functions that both look like this:

def organization_rss_item_count_to_include

if organization_rss_item_count < 5
  organization_rss_item_count
elsif parent_rss_item_count < 5
  10 - parent_rss_item_count
else
  5
end

end

In addition to using if/elsif/else here, the #parent_rss_item_count_to_include method is exactly the same code as the above method except the organization and parent roles are flipped. I feel like there must be a better way to do this without two mirror-image methods, both of which have this kind of control flow. Does anyone have an idea of how I can refactor this into something more elegant?

Thanks!
Geoff


(Jon Seidel) #2

@geoffharcourt There’s a “decompose conditional” refactoring (See the “Refactoring” book) that you might be able to use. You start off by Extracting Methods from the ‘then’ and ‘else’ parts.You might wind up with something like this (guessing at the meaning of each of the sections – you’ll be able to name them better

if small_organization(organization_rss_item_count) then
  small_organization_count(organization_rss_item_count)
elsif small_parent_organization(parent_rss_item_count) then
  small_parent_organization_count( parent_rss_item_count)
else
  default default_count
end

You can then reuse these methods in the other other method and possibly reuse them elsewhere. It doesn’t get rid of the conditional but, at the very least, your code will be clearer as to its intent and you may, as a result, find other refactorings that you can apply.

I’m also interested in what other folks might come up with.