Refactoring IF, a real exercise
Tags: javascript, refactor, performance, challenge
Published: 2021-04-02 18:36:42

## Introduction
Hello, this article is based on a real refactor situation of a javascript software argentine team.

At work we found a code aka "legacy", this made the team of developers go crazy :astonished: :scream:.

There was only one way to solve this, a **refactoring competition :sunglasses::muscle:**.

<img src="https://gecon.es/wp-content/uploads/2016/11/Challenge-Accepted-Meme-3.jpg" width="400px" align="right" />

## The challenge

Five developers were the competitors:
- @EddOVj
- @pabsource 
- @adelysalberto
- @cris_198422
- @pascualmj 
 
and many other spectators.

The goal was to refactor this evil code:

```javascript
const get_data_type = code => {
  
    if (
        code === 702 ||
		code === 1082 ||
		code === 1083 ||
		code === 1114 ||
		code === 1184 ||
		code === 1266 ||
		code === 12403
    ) {
        return "DATE";
    } else if (
        code === 20 ||
		code === 21 ||
		code === 23 ||
		code === 24 ||
		code === 26 ||
		code === 700 ||
		code === 701 ||
		code === 790 ||
		code === 1700 ||
		code === 2202 ||
		code === 2203 ||
		code === 2204 ||
		code === 2205 ||
		code === 2206 ||
		code === 3734 ||
		code === 3769 ||
		code === 12396
    ) {
        return "NUMERIC";
    } else {
        return "STRING";
    }
};
```


Evaluation points:
- semantics
- performance

## The contest prize

None just the **HONOR** and respect from all and and the opportunity to participate in this article of dev.to, because we don't have a budget for these things hahaha.

<img src="https://www.icegif.com/wp-content/uploads/leonardo-di-caprio-cheers-icegif.gif" width="600px" align="right" />


## 3, 2, 1, go!
These were the proposals presented by the participants:

**@pabsource - Pablo**:
```javascript
const codeDate = [20, 21, 23, 24, 26, 700, 701, 790, 1700, 2202, 2203, 2204, 2205, 2206, 3734, 3769, 12396];
const codeNumeric = [702, 1082, 1083, 1114, 1184, 1266, 12403];
const codeInput = 505;

const get_data_type = (codeInput)=>{
  const codeResponse = [codeInput].reduce((acc, item) => {
    const date = codeDate.includes(item) && 'DATE'
    const numeric = codeNumeric.includes(item) && 'NUMERIC'
    return date ? date : numeric || acc
  }, 'STRING');
}
```
> **Analysis**:
> He creates an array and a reducer and includes to get the value of the 
> type and then a ternary to return a default.
> 
> **Semantic**:
> It has a complicated semantics implies mastering the use of reduces and includes.
> 
> **TIME**:
> get_data_type(20); - **0.068115234375 ms**
> get_data_type(702);- **0.008056640625 ms**
> get_data_type(0);  - **0.0068359375 ms**

**@adelysalberto - Adelys**:
```javascript
const codeNumeric = [702, 1082, 1083, 1114, 1184, 1266, 12403];
const codeDate = [20, 21, 23, 24, 26, 700, 701, 790, 1700, 2202, 2203, 2204, 2205, 2206, 3734, 3769, 12396];
const get_data_type = (code) => {
  const codeNum = codeNumeric.includes(code) && 'NUMERIC';
  const codeD = !codeNum && codeDate.includes(code) && 'DATE';
  return codeNum ? codeNum : codeD || "STRING"
}
```
> **Analysis**:
> He use two includes to get the data type, and a single ternary to return the default.
> 
> **Semantic**:
> It has a simpler semantics, it uses only the same operator and a ternary, the default is understood at first glance.
> 
> **TIME**:
> get_data_type(20); - **0.041015625 ms**
> get_data_type(702);- **0.00390625 ms**
> get_data_type(0);  - **0.00390625 ms**


**@EddOVj - Edu**:
```javascript
const codeNumeric = [702, 1082, 1083, 1114, 1184, 1266, 12403];
const codeDate = [20, 21, 23, 24, 26, 700, 701, 790, 1700, 2202, 2203, 2204, 2205, 2206, 3734, 3769, 12396];
const get_data_type = (code) => codeDate.includes(code)?(
        'Date' 
        ): (
            codeNumeric.includes(code) ? 'Numeric' : 'String'
        );
```
> **Analysis**:
> He two ternarys one as main validator and other to return the default.
> 
> **Semantic**:
> It implies knowing includes, it makes a more intensive user of the ternary generates confusion if it is not mastered.
> 
> **TIME**:
> get_data_type(20); - **0.034912109375 ms**
> get_data_type(702);- **0.001953125 ms**
> get_data_type(0);  - **0.003173828125 ms**

**@cris_198422 - Cris**:
```javascript
const dates = [20, 21, 23, 24, 26, 700, 701, 790, 1700, 2202, 2203, 2204, 2205, 2206, 3734, 3769, 12396];
const numerics = [702, 1082, 1083, 1114, 1184, 1266, 12403];
const get_data_type = (code) =>
  dates.includes(code)
    ? 'DATE'
    : numerics.includes(code)
    ? 'NUMERIC'
    : 'STRING';
```
> **Analysis**:
> He use a firts ternary and another ternary to return the default.
> 
> **Semantich**:
> Solution in a line is read as a sentence, it implies mastering concatenated ternaries, it is a little more semantic than the previous one.
> 
> **TIME**:
> get_data_type(20); - **0.03369140625 ms**
> get_data_type(702);- **0.0048828125 ms**
> get_data_type(0); - 0.032958984375 ms

**@pascualmj - Manu**:
He did'nt present a code but communicate an idea, the code was written by me, as an exception it is accepted for this article.
```javascript
const NUMBER='NUMERIC';
const DATE='DATE';
const dataValues = {
  20:DATE,21:DATE,23:DATE,24:DATE,26:DATE,700:DATE,701:DATE,790:DATE,
  1700:DATE,2202:DATE,2203:DATE,2204:DATE,2205:DATE,2206:DATE,3734:DATE,
  3769:DATE,12396:DATE,702:NUMBER,1082:NUMBER,1083:NUMBER,1114:NUMBER,
  1184:NUMBER,1266:NUMBER,12403:NUMBER
};

const get_data_type = (code)=>dataValues[code]||'STRING';
```
> **Analysis**:
> Well this is a very different solution, I have decided not to use includes instead it creates a map to avoid doing array operations and the default is when a value is not found in the map.
> 
> **Semantic**:
> Easy to understand solution does not imply knowing array prototypes, only the operator in the default.
> 
> **TIME**:
> get_data_type(20); - **0.02587890625 ms**
> get_data_type(702);- **0.0029296875 ms**
> get_data_type(0);  - **0.002197265625 ms**


#Conclusion:
Comparing all the cases, the more performance codes:

| Code        | Author | Time |
| ----------- | ----------- | ----------- | 
| get_data_type(20);    | manu     | 0.02587890625 ms        |
| get_data_type(702);   | edu        | 0.001953125 ms        |
| get_data_type(0);   | manu        | 002197265625 ms        |


**The most efficient solution is the one that uses a map to obtain the results created by @pascualmj with a good semantic**.

The solution created by @cris_198422 and @EddOVj they have a good performance too.

The solution created by @adelysalberto have a good performance for date values but suffer in the default and the numeric scenario, but I think that not having used ternary concatenation and a simple default like the includes gives you the best semantic.


# Learnings:
- Use a object map or the JS MAP for this type of scenario.
- Avoid using unnecessary array operations.
- Avoiding the if / else with two ternarys is efficient but we lose semantics.
- Concatenating ternaries can be dangerous if you do not master their use.