How i can dry this code rather than put many conditions

Question

i'm using ROR and in my controller function i recived params and base of these params i need to perform action according condition. But i see these are about 18 conditions.

How can i dry this code.

if params[:topic] == "Topic (title)" and params[:sort] == "Date (ASC)"
  # custom code
elsif params[:topic] == "Topic (title)" and params[:sort] == "Date (DESC)"
  # custom code
elsif params[:topic] == "Topic (title)" and params[:sort] == "Topic (ASC)"
  # custom code
elsif params[:topic] == "Topic (title)" and params[:sort] == "Topic (DESC)"
  # custom code
elsif params[:topic] == "Topic (title)" and params[:sort] == "Author (ASC)"
  # custom code
elsif params[:topic] == "Topic (title)" and params[:sort] == "Author (DESC)"
  # custom code
elsif params[:topic] == "Post (body)" and params[:sort] == "Date (ASC)"
  # custom code
elsif params[:topic] == "Post (body)" and params[:sort] == "Date (DESC)"
  # custom code
elsif params[:topic] == "Post (body)" and params[:sort] == "Topic (ASC)"
  # custom code
elsif params[:topic] == "Post (body)" and params[:sort] == "Topic (DESC)"
  # custom code
elsif params[:topic] == "Post (body)" and params[:sort] == "Author (ASC)"
  # custom code
elsif params[:topic] == "Post (body)" and params[:sort] == "Author (DESC)"
  # custom code
elsif params[:topic] == "Author" and params[:sort] == "Date (ASC)"
  # custom code
elsif params[:topic] == "Author" and params[:sort] == "Date (DESC)"
  # custom code
elsif params[:topic] == "Author" and params[:sort] == "Topic (ASC)"
  # custom code
elsif params[:topic] == "Author" and params[:sort] == "Topic (DESC)"
  # custom code
elsif params[:topic] == "Author" and params[:sort] == "Author (ASC)"
  # custom code
elsif params[:topic] == "Author" and params[:sort] == "Author (DESC)"
  # custom code
end

Many many thanks

custom code will be diffrnt according any condition.first i will search records in DB and then sort according to recieved params — Kashiftufail, Feb 20 '13 at 14:04
As others say, the key to refactoring this code lies in `custom code`. It cannot be refactored well without that being known. — sawa, Feb 20 '13 at 15:43

score 1 · Answer 1 · answered Feb 20 '13 at 14:07

1

Use the case... when syntax. Will make things a bit clearer. Also move the first part of all ifs out of the cases - they seem to be common for quite a few cases.

answered Feb 20 '13 at 14:07

Ivaylo Strandjev

69,226
18
123
176

score 1 · Accepted Answer · answered Feb 20 '13 at 15:40

If the code is legitimately different, consider a case statement:

case [params[:topic], params[:sort]]
when ["Topic (title)", "Date (ASC)"]
  # custom code
when ["Topic (title)", "Date (DESC)"]
  # custom code
when ["Topic (title)", "Topic (ASC)"]
  # custom code
when ["Topic (title)", "Topic (DESC)"]
  # custom code
when ["Topic (title)", "Author (ASC)"]
  # custom code
when ["Topic (title)", "Author (DESC)"]
  # custom code
when ["Post (body)", "Date (ASC)"]
  # custom code
when ["Post (body)", "Date (DESC)"]
  # custom code
when ["Post (body)", "Topic (ASC)"]
  # custom code
when ["Post (body)", "Topic (DESC)"]
  # custom code
when ["Post (body)", "Author (ASC)"]
  # custom code
when ["Post (body)", "Author (DESC)"]
  # custom code
when ["Author", "Date (ASC)"]
  # custom code
when ["Author", "Date (DESC)"]
  # custom code
when ["Author", "Topic (ASC)"]
  # custom code
when ["Author", "Topic (DESC)"]
  # custom code
when ["Author", "Author (ASC)"]
  # custom code
when ["Author", "Author (DESC)"]
  # custom code
end

If there is repetition in the code, or you wind up using this case statement in multiple locations, then there are probably better ways to do this.

Even with the code being custom for each case, you could still clean this up by doing a case for each on `params[:topic]` then a (sub)case under each of those on `params[:sort]`. That would avoid a lot of duplicate references to "Topic (title)", "Post (body)", "Author" and would definitely limit the number of arrays created when trying to process it. Might want to benchmark and look at GC allocation to be sure I'm right, though. — Gary S. Weaver, Feb 20 '13 at 15:47
Yeah, but it would be at the cost of nesting, and unless these are highly volatile, I think it is better to flatten it out and repeat "Topic (title)" etc, because it's just way easier to go down a list and find the right one than it is to follow the nestings in and out. — Joshua Cheek, Feb 23 '13 at 21:54

score 0 · Answer 3 · answered Feb 20 '13 at 14:23



  topics = ['Topic (title)', 'Post (body)', 'Author' ]
  sorts = ['Date (ASC)', 'Date (DESC)', 'Topic (ASC)', 'Topic (DESC)']
  code_map = {
    [topics[0], sorts[0]] => ->() {
      # custom code
    },
    [topics[0], sorts[1]] => ->() {
      # custom code
    }
  }

  # usage
  code_map[[params[:topic], params[:sort]]()

Although that removes the duplication, I don't believe you want to go down this path as I believe the 'custom code' will have a lot of duplication as well.

Show us your 'custom code' and how it varies from one case to the next, and we can dry that for you instead of at the dispatch level

How i can dry this code rather than put many conditions

3 Answers3