Writing clean, readable, and complex application logic in Crystal Language

Nathaniel at Engineering here – when working on the Universal Layer LLC Customer Panel one of the challenges that came up was writing logic that was very complex while remaining clean and readable. It’s very easy to write code that while it works, is very messy and hard to read. Style is just as important as functionality if you plan on making changes later on.

Crystal’s compiler is very strict, to avoid runtime errors later on

One of the benefits of switching to Crystal Language, as well as a challenge to writing code in it, is that the compiler is very strict and does a lot of checks on your code before it compiles. This prevents a lot of common runtime errors that could occur with the same logic in Ruby.

A messy reset method for Virtual Machines

Consider the following method, it has a lot of things to do before we’re sure it’s safe to reset a Virtual Machine. It has to do the following:

• Get a reference to the current_user entity, every time an entity is changed it’s copied to a new space in memory, the old entity is still available if a variable that references it. Once all references to the old entity are gone, the garbage collector will delete the data and allow the space in memory to be used again. Since the current_user entity is subject to change at anytime that code is running, we need to get a reference to the current point in time. We have to do the same with getting the user’s ID number (due to the implementation it’s unlikely to change, but semantics matter a lot while programming).
• Get a reference to the VirtualMachine entity. This contains various information about the machine so we can contact the correct hypervisor and reset the Virtual Machine.
• Get a reference to the service entity. The service entity stores information about billing status, any locks on the service, among other things that could apply to entities other than VirtualMachines.
• Check that there is not a read_only status on the service (we also check if a service is disabled, terminated, etc – it was removed from this example to keep it relatively simple).
• Request the VirtualMachine entity’s reset_vm method to launch. The entity has it’s own methods and logic to make this work for us and avoids rewriting a lot of code. For example a ULayer employee might need to help a customer who contacts us and resetting the Virtual Machine will be one of the basic troubleshooting steps we take. Writing the logic twice makes the code messier and harder to change later. We’re discussing internally whether status checks to prevent an action should be addressed by the entity or the controller. We could add an optional parameter to overwrite the check. For now though, we’ve chosen to address this in the controller.
• Create an alert (internally this system is called flash messages)
• Redirect the user. During the alpha all redirects went to the homepage, and improvements are ongoing to our use of redirects.

def reset
if user = current_user
            if user_id = user.id
                if server = VirtualMachine.find_by(user_id: user_id, id: params[:server_id])
                    if service = Service.find_by(id: server.service_id)
                        unless service.service_is_read_only
                            server.reset_vm
                            flash[:info] = "Server reset!!"
                            redirect_to "/"
                        end
                    end
                end
            end
        end

end

Needless to say this code is a bit messy. What happens if we need to start referencing and using other entities or adding in extra logic to this method? Making this readable becomes a top priority.

Our solution

We found a really neat alternative to nested if-statement logic we were using. Similar situations with lots of functions relying on each other can occur in JavaScript, commonly referred to as “callback hell” and this reminded the team of it. Consider the following code:

def reset
    if (user = current_user) && (user_id = user.id) && (server = VirtualMachine.find_by(user_id: user_id, id: params[:server_id])) && (service = Service.find_by(id: server.service_id))
      unless service.service_is_read_only || service.service_is_disabled
        server.reset_vm
        flash[:info] = "Server reset!!"
        redirect_to "/services/virtual_machines/#{server.id}"
      end
    end
  end

This new approach creates a single if-statement, we evaluate several isolated expressions (and they’re evaluated in order using the && operator), if they all evaluate to a truthy value (e.g. not false or nil), the code block runs (we will return an error screen in the event of a failure in a future version of our application). We still decided to nest an unless-statement (unless-statements run unless the expression evaluates to a truthy value. It’s easier to read than the if-statement !expression (you see to accomplish this in other languages) that checks the service status flags. Since we’re considering moving the service status flag checks into the VirtualMachine entity this design decision makes our code readable and easier to change later on.

Drawbacks

No solution to writing clean code is perfect. Some of the drawbacks of our approach include:

• The code is more readable and avoids nested if-statements. That said the expression evaluation chain is quite long and could still be challenging to read later on.
• Several expressions must evaluate before the code block is run. In most cases they evaluate quickly however if we wanted to call in data from a remote API, execution of the code block would be deferred until data was returned.
• We might want to move service status flag checks to the entity, although there are several considerations to make before we’re ready to do this.

Conclusion

There are many things to consider when writing clean code and it varies depending on the purpose of the application and the applicable logic. We’re happy with the success our current approach has given and look forward to improving it even more. In the meantime, check out our panel :).