Re-factoring

Always wanted to travel back in time to try fighting a younger version of yourself? Software development is the career for you!
-@Loh

It’s so weird to come back to something you were happy with, so happy with you posted it to your site, and say ‘gah, this is terrible.’ There were so many things wrong with that last calculator.

It’s super important to say here that there is no absolute way to determine whether code is good or bad. Beyond ‘working’ vs. ‘not working’ there are a few other standards: code that is easy for most other developers to understand, that handles error conditions in a predictable way, code that uses fewer server resources to run. Every thing except ‘doesn’t run’ is just a matter of degree, and often you gain performance by sacrificing readability etc. etc. My gripes about my own code are more a list ‘red flags’ than any absolute problem

Let’s talk about some ways to evaluate code that is in general, not so good:

  • tons of comments – comments are good and helpful when they are good and helpful. If every third line of your code needs explanation, there’s a chance your code path is pretty convoluted

  • multiple safeties – my first version had two different ‘checker’ methods for a few dozen code lines. Two methods to check one line of user input isn’t great :/ At a basic level: methods have error conditions already, or ways to handle weird input. Double-triple-checking that you’re not calling pop() on an empty array isn’t actually necessary, just make your other functions handle undefined.

  • long – I’ve talked about this before, hell everyone has talked about it, but there’s usually a good chance that going from 60 to 12 lines for the same functionality will be easier to read and understand.

so here’s the ‘better Reverse Polish Calculator’:

var stack = []
function handleInput(expressions){
  if (expressions == "q"){
    console.log("recieved 'q,' quitting")
    process.exit(0)
  }
  var inputArray = expressions.split(' ')
  inputArray.forEach(function (expression){
    if (/\d/.test(expression)){
      stack.push(parseFloat(expression))
    }
    switch (expression) {
      case '+':
        stack.push(stack.pop() + stack.pop())
        break
      case '-':
        stack.push(stack.pop() - stack.pop())
        break
      case '*':
        stack.push(stack.pop() * stack.pop())
        break
      case '/':
        stack.push(stack.pop() / stack.pop())
        break
    }
    console.log(stack)
  })
}


process.stdin.resume()
process.stdin.setEncoding('utf8')
process.stdout.write('\033[32m> \033[0m') //input prompt
process.stdin.on('data', function(datum) {
  handleInput(datum.trim())
  process.stdout.write('\033[32m> \033[0m')
});


One thing I love about this refactoring is it doesn’t actually do anything new. I did some research into whether it would be possible to just use the "+" as an operator without having to recognize the string, but there was no easy way to get it done.

My own Postfix Calculator

Job hunt time and that means writing some code to do random stuff to things!

Given the requirements that the code to do this be ‘readable, easy to extend, and understand’ I wrote a Reverse Polish Notation calculator.

//reminder: 'operand' == some kinda number, 'operator' == a math symbol
var operandCount = 0 //for checking if we have > 2 operands to do stuff to
var stack = [] //list of everything entered, with operands as floats and operators as strings
var validCharacters = ['1', '2', '3', '4', '3', '4', '5', '6', '7', '8', '9', '10',
' ', '*', '+', '/', '-' ]
function handleInput (datum){
  if (datum == 'q'){
    console.log("recieved 'q,' quitting")
    process.exit(0);
  }
  var response = ''
  if (!checkInvalid(datum)){
    console.log ('Sorry, you can only enter '+validCharacters)
  } else { //the input wasn't invalid characters
    datum = datum.replace(/^\s*|\s*$/, '') //Pattern's just removing initial or terminal whitespace
    //if more thn one value was given at once, grab each
    //TODO: if you enter more than one value the 'operandCount' checker will
    //become inaccurate. Basically the entering of multiple values isn't fully
    //supported in this version.
    datum.split(' ').forEach(function (value){stack.unshift(value)})
    var lastVal = stack.shift()
    if (isNumeric(lastVal)){
      //console.log('oh my, that's a number')
      operandCount++
      stack.unshift (parseFloat(lastVal))
    } else{
      if (operandCount < 2){
        console.log ('please enter '+(2-operandCount)+' more value!')
      } else {
        operandCount -- //one operand is destroyed in any function
        switch (lastVal){
          case '+' : performAddition() ; break
          case '/' : performDivision() ; break
          case '-' : performSubtraction() ; break
          case '*' : performMultiplication() ; break
        }

      }
    }


  }
  response = stack[0]
  console.log (response)
}

//this function could be written on one line with functional JS but it would be harder to read
function checkInvalid(datum){
  var reply = true
  datum.trim().split('').forEach(function(character){
    if (validCharacters.indexOf(character) < 0){
      reply = false
    }
  })
  return reply
}

function isNumeric(n){
  n = parseFloat(n) //everything's coming in as a string from stdin.on('data') :shrug emoji:
  return (typeof n == 'number' && !isNaN(n))
}
//are you at all worried I'm shift()ing the operator here? nope! I shifted it already
function performAddition (){
  stack.unshift(stack.shift() + stack.shift())
}

function performSubtraction (){
  stack.unshift(stack.shift() - stack.shift())
}

function performMultiplication (){
  stack.unshift(stack.shift() * stack.shift())
}

function performDivision (){
  stack.unshift(stack.shift() / stack.shift())
}





process.stdin.resume();
process.stdin.setEncoding('utf8');
process.stdout.write('\033[32m> \033[0m'); //input prompt
process.stdin.on('data', function(datum) {
  handleInput(datum.trim())
  process.stdout.write('\033[32m> \033[0m');
});

Install

  1. Make Node work on your system
  2. download this script to TFPostfixCalc.js
  3. run node TFPostFixCalc.js
  4. enter numbers or operators into the prompt q or EOF should both quit

TODO

In order to seem like a real human being I left one of the issues as a TODO in the code, because if you enter 10 42 the code will sorta work, but to improve it I’d also have to handle 10 * where I’m not totally clear on for the standard… I think it’s just an error?

Other stuff I don’t love:

  • all those separate functions for the operators could at least be written more densely
  • several checks could be rewritten with chaining and Functional patterns to be way more dense, but this is the way I write it first and I think this ‘fluffy’ version is easier to read
  • I feel it’s correct on bad input to just ignore the last entered line and maintain the state otherwise. Wasn’t in the spec, just said ‘won’t accept junk’ so :shrug emoji:. The other option would be to quit at that point.
  • Might want to color the output? IDK hardly major…