CODEWITHSUNDEEP
rubyoopmethodssimplify
user15788830
user15788830

Reputation:

Is it possible to simplify this method?

I'm using a test method to model a method in a cli tool I'm making, in order to work on the method in isolation. It's a very simple method and I'm trying to keep it as simple as possible. I've managed to simplify the method to this:

LOGS = [{ "first_log" => ["first task"] }, { "second_log" => ["second task"] }, { "third_log" => ["third task", "additional task"] }]

LOG_NAME = "second_log" 

def output
  LOGS.each { |log| 
    if LOG_NAME
      puts "-#{log.keys.first.to_s}:", " Tasks: #{log.values.join(",")}" if log.key?(LOG_NAME)
    else
      puts "-#{log.keys.first.to_s}:", " Tasks: #{log.values.join(",")}" 
    end
    }
end

The aim here is that when the function is called, if there is a LOG_NAME present it outputs just that log (hash object) and its tasks. If no LOG_NAME is present it outputs all logs (hash objects) in the LOGS array.

This works fine, but my question is: is there a way to make this method even simpler? I feel like I can get rid of the if/else logic and streamline it more but I just cannot conceive of how. I've also tried using rubocop but it 1) suggests similar if/else logic 2) suggests corrections that don't actually output anything.

Any ideas are greatly appreciated, thank you.

Upvotes: 0

Views: 80

Answers (2)

Surya
Surya

Reputation: 15992

Why not just skip the iteration for the matching condition?

def output
  LOGS.each do |log|
    next if LOG_NAME && log.key?(LOG_NAME)
    puts "-#{log.keys.first.to_s}:", " Tasks: #{log.values.join(",")}" 
  end
end

This makes it easier to digest the code that if LOG_NAME and log's key has LOG_NAME in it then skip, else continue the execution.

Upvotes: 0

ptd
ptd

Reputation: 3053

I think you could improve this code by separating the filtering of the logs from the printing of the logs. This approach reduces duplication and I think better describes the problem.

LOGS = [{ "first_log" => ["first task"] }, { "second_log" => ["second task"] }, { "third_log" => ["third task", "additional task"] }]

LOG_NAME = "second_log" 

def output
  LOGS.select { |log|
    LOG_NAME ? log.key?(LOG_NAME) : true
  }.each { |log| puts "-#{log.keys.first.to_s}:", " Tasks: #{log.values.join(",")}"  }
end

Upvotes: 1

Related Questions