A memorandum to clean up the code Ruby

Writing code neatly is difficult

I don't notice it when I'm writing it, and even if I look at it after I finish writing it, it's difficult to disassemble and reconstruct the code I wrote. That's because you need a different approach to writing code when rebuilding. The place where I once thought that this was the case and rewrote the code with a blank head is the mountain. This time, I will write out some patterns that I noticed in such a rewritten part. It is a memorandum based on my own experience.

Almost the same processing is included in the conditional expression

Actually, the process you want to do is the same, but there are two types of param contents that are passed as arguments, on_total and on_this_month, and if you detect either of those two, you wanted to include a process to sort. The first one I wrote is below.


def self.ascend_checkins(param, hash)
  if param == "on_total"
    hash.sort_by { |k,v| -v["total"] }.to_h
  elsif param == "on_this_month"
    hash.sort_by { |k,v| -v["month"] }.to_h

For the reason mentioned above, the same sort process has been included, so modify it as follows.


  def self.ascend_checkins(hash, param)
    key_of = {"on_total" => "total", "on_this_month" => "month"}
    value = key_of[param]

    raise "unknown param" if !value
    hash.sort_by { |k,v| -v[value] }.to_h

Since there are only two params, prepare a hash, receive the param, judge the value corresponding to it by the boolean value, and if it is true, delete the same process by executing the process.

Many temporary variables

The following is the process of extracting the user ID as a character string, counting the number of it, and converting it into a hash. It has become a process with many temporary variables when preparing an empty array.


def self.get_monthly_count(ids, users)
    uids = []
    ids.each do |id|
      uids.push id.uid.to_s

    uid = {}
    count = 0
    users.each do |user|
      uid[user] = uids[count]
      count += 1

It's tidy up in one line, in the case of Ruby.


def self.get_monthly_count(ids, users)
  users.zip(ids.map{ |id| id.uid.to_s }).to_h

First, if you use the map method, you only need one line to extract it as a character string and recreate it as an array. If you create an empty array and write processing using indexes, it seems that you can write clean code from the beginning if you think of using this kind of method first. After that, I use the zip method to create a pair array and finally hash it.

zip pattern

It is a method that can combine elements of an array like a chuck.


array_keys = ["Tom", "Bob", "Joge"]
array_values = [167, 156, 181]

zipped = array_keys.zip(array_values)
p zipped # [["Tom", 167], ["Bob", 156], ["Joge", 181]]

hash = zipped.to_h

p hash # {"Tom"=>167, "Bob"=>156, "Joge"=>181}

Many temporary variables

There are three, user_names, uid, and users, but you don't need to create them like this.


def self.get(check_ins)
  user_names = []
    check_ins.each do |check_in|
      uid = check_in.uid
      users = community_number_collection.document(uid).get
      user_names.push users.data[:nickname]

If you have an empty array, use map to process it, and put it on one line without creating extra variables.


def self.get(check_ins)
  check_ins.map do |check_in|

Combine map and zip to shorten

Combine the ones mentioned above to shorten the process.


def self.get_total(merged_data, users)
    sum =  []
    merged_data.keys.each do |value|
      uid = merged_data[value]["uid"]
      sum.push self.where(uid: uid).count
    total = {}
    count = 0
    users.each do |user|
      total[user] = sum[count]
      count += 1

Map and zip will make it much shorter. It was also a discovery that the entire block processing was pushed into a variable called sum. The block is an expression, and it will be possible to assign it.


def self.get_total(merged_data, users)
  sums = merged_data.keys.map do |key|
    self.where(uid: merged_data[key]["uid"]).count

Better hash math

Hashes count duplicates and are useful for aggregating. But the shorter it is, the better.


def self.get_this_month(check_ins)
    check = {}
    check_ins.each do |check_in|
      uid = check_in.uid.to_s
      uid_count = check[uid] || 0
      check[uid] = uid_count + 1
    check.values.each do |value|

I haven't used an empty array this time, but if you put the process of extracting the elements of the original array into each, it's better to recreate the desired array with map and turn that array with each. .. Also, if you use the default method, you can set all values to 0 once.


def self.get_this_month(check_ins)
  check = {}
  check.default = 0
  check_ins.map{ |c| c.uid.to_s }.each do |uid|
    check[uid] += 1

Function generalization

Before the code below, there were two functions that did the same thing, except that the keys of the hash passed were different. I put it together in one abstract function. After preparing an empty array, declare an empty array with key specified (nested = {}), and in each, a hash with no value is created for key (nested [key] = {} ).


  def self.label_hash(hash_dictionary, keys)
    nested = {}

    keys.each do |key|
      nested[key] = {}
      hash_dictionary.each do |name, hash|
        nested[key][name] = hash[key]

As a calling method, the method of grouping the arguments into a hash is used. In the upper function, the words used are close to abstract, and in the lower call, words with strong concreteness are used. You can see that the roles are clearly separated by naming.


labeled_totally_count = CheckIn.label_hash({
    "month" => zipped_monthly_count,
    "total" => zipped_totally_count
}, zipped_monthly_count.keys)


--When there are many temporary variables, imagine how to write them erased.

--Be aware of the moment you use the method provided by Ruby depending on the situation (especially if you are writing an empty array and index counting process)

Another problem with writing code that is difficult to read is that the name you are already using is rooted in the programming world as a different concept, so a skilled engineer can see the code. Be aware that it can often be recalled at the end, leading to misreading, and as a result, the code you write is hard to read.

Recommended Posts

A memorandum to clean up the code Ruby
[Ruby] Code to display the day of the week
AtCoder Beginner Contest 170 A, B, C up to ruby
[Ruby] How to retrieve the contents of a double hash
A memorandum to reach the itchy place for Java Gold
A memorandum for writing beautiful code
A memorandum of the FizzBuzz problem
I tried to write code like a type declaration in Ruby
(Ruby on Rails6) Create a function to edit the posted content
A memorandum on how to use Eclipse
The road to creating a music game 2
[Ruby] From the basics to the inject method
Minimal steps to set up a Ruby environment with rbenv on Ubuntu 20.04
[Ruby] I tried to diet the if statement code with the ternary operator
I thought about the best way to create a ValueObject in Ruby
[Ruby] Returns characters in a pyramid shape according to the entered numbers
The road to creating a music game 3
Set up a Java GUI in a separate thread to keep the main
The road to creating a music game 1
Why was the code painful to read?
[Java: memorandum] Until the line feed code CRLF is changed to LF
If it is Ruby, it is efficient to make it a method and stock the processing.
03. I sent a request from Spring Boot to the zip code search API
Up to the point of launching a Docker container built using RedHat Quarkus
[Ruby] Sending a POST HTTP request to the Web API using OAuth authentication
"Mathematical puzzle to train the program brain more" _Q40 (code: Ruby)-> Rust unfinished
How to find the cause of the Ruby error
Make a margin to the left of the TextField
Code to escape a JSON string in Java
The story of introducing Ajax communication to ruby
Set the time of LocalDateTime to a specific time
[Ruby] I want to do a method jump!
The code I used to connect Rails 3 to PostgreSQL 10
[Ruby on Rails] A memorandum of layout templates
How to write code that thinks object-oriented Ruby
[Ruby] When adding a null constraint to a table
[Ruby] How to generate a random alphabet string
Clean up findViewById from source code with DataBindingLibrary
To get Ruby Silver at the last minute
How to build the simplest blockchain in Ruby
[Ruby] Setting values ​​and memorandum for the table
I want to get the value in Ruby
[Ruby basics] How to use the slice method
The road to creating a Web service (Part 1)
[Ruby] I want to make a program that displays today's day of the week!
Introduction to Ruby 2
[Ruby] Method memorandum
[Object-oriented] A memorandum that makes zero the best [Site summary that helped to understand the concept]
[ruby] How to assign a value to a hash by referring to the value and key of another hash
Writing code Ruby
Sample code to assign a value in a property file to a field of the expected type
A memorandum because I was addicted to the setting of the Android project of IntelliJ IDEA
A review of the code used by rails beginners
[Ruby] Basic code list. Keep the basics with examples
Count the number of occurrences of a string in Ruby
[Introduction] Try to create a Ruby on Rails application
A brief introduction to terasoluna5, see the text below
How to run the SpringBoot app as a service
I read the readable code, so make a note
[Ruby] Generate a concatenated QR code with rqrcode (Knowledge)
A memorandum of addiction to Spring Boot2 x Doma2