<img src="https://secure.leadforensics.com/794635.png" style="display:none;">
Skip to content
Published at March 31, 2022
Software Engineering

Code Health: simple ways to avoid complexity

If you've used Virtual Code Review or PR integration, you already know that complexity is a frequent target of CodeScene's Code Health penalties. 

If you have used CodeScene's Virtual Code Review or PR integrations, then you already know that complexity is a frequent target of CodeScene's Code Health penalties. 

 

Just one or two extra if / then blocks around or inside a function will quickly lead to a warning on a PR.

Some conditionals are directly related to the task at hand, to the business logic. Others are more "accidental", determined by language or implementation details. In Clojure, checking for nil can be one of those details.

Consider this example.

A service we consume produces a string, with key-value pairs. The pairs are separated by semi-colons; each key and value are separated by a colon:

status:1203;statusText:pending;startIndex:4309


Let's say we want to check the status. We write some code to parse the string, splitting on
 ; and then on :. And finally returning the status code.

Eventually, we realize that we probably want all the keys, so we write a function to extract everything into a map:

(defn parse-message [message-str]
  (let [kv-pairs (str/split message-str #";")]
    (->> kv-pairs
         (map #(str/split % #":"))
         (into {}))))

 

But what if the string is empty? What if it's not even there, and we have nil instead? Clojure's standard clojure.string/split throws an exception on nil.

Execution error (NullPointerException) at java.util.regex.Matcher/getTextLength (Matcher.java:1769).
Cannot invoke "java.lang.CharSequence.length()" because "this.text" is null

 

Our reflex is to add a defensive check:

(when message-str
  ( ;; parse our stuff)
   ))


Then we realize that we're maybe passing along a
 nil ourselves, which will force the caller to deal with that too.

So this, then?

(if message-str
  ( ;; parse our stuff)
   )
  {}) ;; or pass on an empty map

(Let's assume for the purpose of this example that we don't care about reporting errors: if the input is bad we just want an empty map.)

 

There's a second check we need to perform as well though. What if kv-pairs doesn't contain... pairs?

user> (parse-message "not pairs")
Execution error (IllegalArgumentException) at user/parse-message (form-init14991840198581339031.clj:187).
Vector arg to map conj must be a pair
 
 

I guess we need to add another check...

(if (and message-str (message-str-is-valid? message-str))
  ( ;; parse our stuff))
  {}) ;; or pass on an empty map
 

We're safe now, right?

Sort of.

We've avoided a null pointer exception but our code is now nested inside a conditional. In this simple case it wouldn't matter much, but in more complex situations this additional complexity would indeed make the function harder to read. "What's that extra check…? Oh yeah, the nil." If you add enough of these, CodeScene will start to complain about complexity on your PR. We're also potentially passing a nil onto the caller, who will have to deal with the same problem, one way or another.

What if we could just rewrite clojure.string/split to change the way it handles nil? Well… we can: that's what fnil is for.

If you don't remember fnil, it's a core Clojure function that wraps a function and basically intercepts a nil argument and replaces it with whatever default you provide.

That gives us this:

(defn parse-message [message-str]
  (let [kv-pairs ((fnil str/split "") message-str #";")]
    (->> kv-pairs
         (map #(str/split % #":"))
         (into {}))))

 

To solve the second problem, with the key value pairs, we can just filter out anything that's not a pair:

(defn parse-message [message-str]
    (let [kv-pairs ((fnil str/split "") message-str #";")]
      (->> kv-pairs
           (map #(str/split % #":"))
           (filter #(= (count %) 2))
           (into {}))))
 

It's the same function, with the behavior we want, one extra function call, and no additional complexity. CodeScene's happy now, and so are you.

Keep on reading! We've been working on better refactoring tools for teams. 

Joseph Fahey

Elements Image

Subscribe to our newsletter

Lorem ipsum dolor sit amet, consectetur adipiscing elit. Semper neque enim rhoncus vestibulum at maecenas. Ut sociis dignissim.

Latest Articles

AI Coding Assistants: Introducing CodeScene AI Generated Code Refactoring

AI Coding Assistants: Introducing CodeScene AI Generated Code Refactoring

AI Coding Assistants: Let's introduce you to AI generated code refactoring. Read more and join the Beta testing program.

Change coupling: visualize the cost of change

Change coupling: visualize the cost of change

Code can be hard to understand due to excess accidental complexity. Or, it can look simple, yet its behavior is anything but due to complex...

CodeScene's IDE Extension brings CodeHealth™ Analysis directly into your editor

CodeScene's IDE Extension brings CodeHealth™ Analysis directly into your...

We've just launched an IDE Extension for VS Code, helping developers tackle code complexity within the editor. Read more and try it out!