My Basic Tax Calculator

Question

The goal is:

• I can add many services as I need.
• I can add many taxes as I need (the idea is that services has all of this taxes)
• Calculate per services each tax value
• Sum all the taxes of each service and throw me the total value per service.

This is the first attempt at making a JavaScript program and I would love to hear any advice, what did I do wrong, what is right, what could be better or improved.

CodePen

// Services
var servicios = [
    //  { nombre_servicio: 'Fotografia', valor: 40000 }, { nombre_servicio: 'Tarjetas', valor: 60000 }
];
var tablaServicios = 0;
var nS = document.getElementById('nombre-servicio');
var vS = document.getElementById('valor-servicio');
var botonAgregarServicio = document.getElementById('agrega-valor');

//Taxes
var impuestos = [
    //{ nombre_impuesto: 'Iva', valor: 6 }, { nombre_impuesto: 'ICA', valor: 2 }
];
var tablaImpuestos = 0;
var nI = document.getElementById('nombre-imp');
var vI = document.getElementById('num-imp');
var botonAgregarImpuesto = document.getElementById('agrega-impuesto');

//Calculate button
var botonCalcular = document.getElementById('calcular');

// Calculate Event on click
botonCalcular.addEventListener("click", calcularImpuestos);

//Add service
botonAgregarServicio.addEventListener("click", agregaServicio);

//Get input values and add to services array
function agregaServicio() {
    servicios.push({
        nombre_servicio: nS.value,
        valor: vS.value
    });

    //Close modal    
    $('#modalServ').modal('hide');
    //Clear inputs 
    $('.agrega-servicio input').val('');

    //Get last item of services array
    var s = servicios[servicios.length - 1].nombre_servicio;
    var v = servicios[servicios.length - 1].valor;

    //Add table if not exist
    if (tablaServicios == 0) {
        genera_tabla('servicios');

        var tabla = document.querySelectorAll('.servicios thead tr th');
        tabla[0].innerHTML = 'Nombre servicio';
        tabla[1].innerHTML = 'Valor servicio'

        agregaFila('.servicios', s, '$ ' + v);

        //Delete "add new service" overlay
        var el = document.querySelectorAll('#servicios .no-agrego');
        el[0].parentNode.removeChild(el[0]);

        tablaServicios = 1;

    } else {
        agregaFila('.servicios', s, '$ ' + v);
    }

}

//Add tax
botonAgregarImpuesto.addEventListener("click", agregaImpuesto);

//Get input values and add to taxes array
function agregaImpuesto() {
    impuestos.push({
        nombre_impuesto: nI.value,
        valor: vI.value
    });

    //Close modal     
    $('#modalImp').modal('hide');
    //Clear inputs
    $('.agrega-impuesto input').val('');

    //Get last item of taxes array
    var s = impuestos[impuestos.length - 1].nombre_impuesto;
    var v = impuestos[impuestos.length - 1].valor;

    //Add table if not exist
    if (tablaImpuestos == 0) {
        genera_tabla('impuestos');

        var tabla = document.querySelectorAll('.impuestos thead tr th');
        tabla[0].innerHTML = 'Nombre impuesto';
        tabla[1].innerHTML = 'Valor impuesto (%)'

        agregaFila('.impuestos', s, v + ' %');

        //Delete "add new tax" overlay
        var el = document.querySelectorAll('#impuestos .no-agrego');
        el[0].parentNode.removeChild(el[0]);

        tablaImpuestos = 1;

    } else {
        agregaFila('.impuestos', s, v + ' %');
    }

}

function calcularImpuestos() {

    if (servicios.length > 0 && impuestos.length > 0) {

        //Loop services array items
        for (var i = 0; i < servicios.length; i++) {

            //Results container
            var resultados = document.getElementById('genResultados');

            //Generate elements in DOM
            var contenedorTabla = document.createElement('div');
            var modulo = document.createElement('div');
            var h1 = document.createElement('h1');
            var tituloServicio = document.createTextNode(servicios[i].nombre_servicio);
            var panel = document.createElement('div');
            var panelBody = document.createElement('div');
            var panelFooter = document.createElement('div');
            var row = document.createElement('div');
            var col1 = document.createElement('div');
            var titR1 = document.createTextNode('Total');
            var col2 = document.createElement('div');

            //Add created elements into container
            col1.appendChild(titR1);
            row.appendChild(col1);
            row.appendChild(col2);
            h1.appendChild(tituloServicio);
            modulo.appendChild(h1);
            modulo.appendChild(panel)
            panel.appendChild(panelBody);
            panel.appendChild(panelFooter);
            panelFooter.appendChild(row);
            panelBody.appendChild(contenedorTabla);
            resultados.appendChild(modulo);

            //assign classes and id to elements
            contenedorTabla.id = 'resultados-' + i + '-servicio';
            modulo.className = 'servicio-' + i;
            panel.className = 'panel panel-default';
            row.className = 'row';
            col1.className = 'col-md-6 col-xs-6';
            col2.className = 'col-md-6 col-xs-6';
            panelBody.className = 'panel-body';
            panelFooter.className = 'panel-footer';

            genera_tabla('resultados-' + i + '-servicio');

            //Add thead
            var tablaHead = document.querySelectorAll('.resultados-' + i + '-servicio thead tr th');
            tablaHead[0].innerHTML = 'Nombre impuesto';
            tablaHead[1].innerHTML = 'Valor del impuesto';

            //Array to store taxes results
            var mArray = []

            //Loop taxes array
            for (var j = 0; j < impuestos.length; j++) {
                var nombreImpuesto = impuestos[j].nombre_impuesto;
                //Tax value
                var k = impuestos[j].valor;

                //Service value
                var l = servicios[i].valor;

                //Math operation get percentage 
                var m = (k * l) / 100;

                //Add [m] result to array 
                mArray.push(m);

                //Add row to html results each tax name and value
                agregaFila('.resultados-' + i + '-servicio', nombreImpuesto, '$ ' + m);

            }

            //Get service module
            var tablaBody = document.querySelectorAll('.resultados-' + i + '-servicio');

            //Sum all taxes per service            
            var sumaImpuestos = 0;

            for (var x = 0; x < mArray.length; x++) {
                sumaImpuestos += mArray[x];
            }

            //Add result to dom
            var titR2 = document.createTextNode('$ ' + sumaImpuestos);
            col2.appendChild(titR2);

        }

        //Delete calculate buton
        var el = document.querySelectorAll('.well #calcular');
        el[0].parentNode.parentNode.removeChild(el[0].parentNode);

    } else {
        alert('No hay valores para calcular');
    }

}

function genera_tabla(id) {
    // Get container element
    var contenedor = document.getElementById(id);
    // Create element <table> and <tbody> <thead>
    var tabla = document.createElement("table");
    var tblBody = document.createElement("tbody");
    var thead = document.createElement('thead');
    var tr = document.createElement('tr');
    var thNombre = document.createElement('th');
    var thValor = document.createElement('th');

    tr.appendChild(thNombre);
    tr.appendChild(thValor);
    thead.appendChild(tr);
    tabla.appendChild(thead);

    // place <tbody> inside <table>    
    tabla.appendChild(tblBody);
    // place <table> inside container
    contenedor.appendChild(tabla);
    //Add bootstrap class to table
    tabla.className = 'table ' + id;
}

//Add row to table
function agregaFila(clase, nombre, valor) {
    //Get table
    var tabla = document.querySelectorAll(clase);
    //Insert row at the end of the table
    var fila = tabla[0].children[1].insertRow(-1);
    //Insert cell 1
    var celda1 = fila.insertCell(0);
    //Insert cell 2
    var celda2 = fila.insertCell(1);
    //Insert data from name
    celda1.innerHTML = nombre;
    //Insert data from value
    celda2.innerHTML = valor;
}

<html class="no-js" lang="">

<head>
  <meta charset="utf-8">
  <meta http-equiv="X-UA-Compatible" content="IE=edge,chrome=1">
  <title></title>
  <meta name="description" content="">
  <meta name="viewport" content="width=device-width, initial-scale=1">
  <style>
    body {
        padding-top: 50px;
        padding-bottom: 20px;
    }
  </style>
  <link rel="stylesheet" href="css/main.css">
  <script src="js/vendor/modernizr-2.8.3.min.js"></script>
</head>

<body>
  <nav class="navbar navbar-inverse navbar-fixed-top" role="navigation">
    <div class="container">
      <div class="navbar-header">
        <a class="navbar-brand center-block" href="#">Taxes</a>
      </div>
    </div>
  </nav>
  <div class="container">
    <div class="row">
      <div class="col-md-12">
        <h2>Services</h2>
        <div class="panel panel-default">
          <div id="servicios" class="panel-body">
            <div class="no-agrego"><span class="glyphicon glyphicon-pencil"></span> Add new service</div>
          </div>
          <div class="panel-footer">
            <a id="agregar-valor" data-toggle="modal" data-target="#modalServ" class="btn btn-lg btn-default btn-block"> <span class="glyphicon glyphicon-plus-sign" aria-hidden="true"></span> Add service</a>
          </div>
        </div>
      </div>
      <div class="col-md-12">
        <h2>Tax</h2>
        <div class="panel panel-default">
          <div id="impuestos" class="panel-body">
            <div class="no-agrego"><span class="glyphicon glyphicon-pencil"></span> Add new tax</div>
          </div>
          <div class="panel-footer">
            <a id="agregar-impuesto" data-toggle="modal" data-target="#modalImp" class="btn btn-lg btn-default btn-block"> <span class="glyphicon glyphicon-plus-sign" aria-hidden="true"></span> Add tax</a>
          </div>
        </div>
        <div class="well">
          <a id="calcular" class="btn btn-lg btn-primary btn-block"> <span class="glyphicon glyphicon-cog" aria-hidden="true"></span> Calculate</a>
        </div>
      </div>
      <div id="genResultados" class="col-md-12">

      </div>
    </div>
    <hr>
    <footer>
    </footer>
    <!-- Modal  Servicio-->
    <div class="modal fade" id="modalServ" tabindex="-1" role="dialog">
      <div class="modal-dialog" role="document">
        <div class="modal-content">
          <div class="modal-header">
            <button type="button" class="close" data-dismiss="modal" aria-label="Close"><span aria-hidden="true">&times;</span></button>
            <h4 class="modal-title">Add service</h4>
          </div>
          <div class="modal-body">
            <div class="agrega-servicio">
              <div class="form-group">
                <label for="nombre-servicio">Service name</label>
                <input type="text" class="form-control" id="nombre-servicio" placeholder="Ex: Logo Design">
              </div>
              <div class="form-group">
                <label for="num-valor">Service value</label>
                <input type="number" class="form-control" id="valor-servicio" placeholder="Ex: 40000">
              </div>
              <button type="submit" class="btn btn-primary" id="agrega-valor">Add</button>
              <button type="button" class="btn btn-default" data-dismiss="modal">Close</button>
            </div>
          </div>
        </div>
      </div>
    </div>
    <!-- Modal  Impuesto-->
    <div class="modal fade" id="modalImp" tabindex="-1" role="dialog">
      <div class="modal-dialog" role="document">
        <div class="modal-content">
          <div class="modal-header">
            <button type="button" class="close" data-dismiss="modal" aria-label="Close"><span aria-hidden="true">&times;</span></button>
            <h4 class="modal-title">Add tax</h4>
          </div>
          <div class="modal-body">
            <div class="agrega-impuesto">
              <div class="form-group">
                <label for="nombre-imp">Tax name</label>
                <input type="text" class="form-control" id="nombre-imp" placeholder="Ex: VAT">
              </div>
              <div class="form-group">
                <label for="num-imp">Tax value (%)</label>
                <input type="number" class="form-control" id="num-imp" placeholder="Ex: 6">
              </div>
              <button type="submit" class="btn btn-primary" id="agrega-impuesto">Add</button>
              <button type="button" class="btn btn-default" data-dismiss="modal">Close</button>
            </div>
          </div>
        </div>
      </div>
    </div>
  </div>
  <!-- /container -->
  <!--   <script src="//ajax.googleapis.com/ajax/libs/jquery/1.11.2/jquery.min.js"></script> -->
  <script src="js/vendor/jquery-1.11.2.min.js"></script>
  <script src="js/vendor/bootstrap.min.js"></script>
  <script src="js/main.js"></script>
</body>

</html>

Answer 1

Advice 1

You break the Single Responsibility principle in calcularImpuestos().

The name, I guess, lead to a function that calculate something. But inside you do other staff like design a part of the HTML and style.

You should move this code inside a different function designed to handle HTML only. And have just calculation inside this function.

Advise 2

You use mainly javascript DOM API to create the UI, this is not so good.

You should write your HTML and use a different approach introducing template engines or write your own short version.

This helps maintainability on the code, as with the plain HTML you could have a better idea about what should appear on the page. And enhancements will also be easy to do.

Another good reason is you can easily divide the view/presentation from the model and the logic. (this is not strictly connected to MVC patterns, instead it is always a good design principle)

Advice 3

In agregaServicio() and agregaImpuesto() you have broke the Don't Repeat Yourself principle.

The code is pretty much the same, you could write a single function that do the task, with few parameters.

You could still have your 2 different functions without parameters, that will call the implementation, with the proper values:

function agregaServicio() {
    return aggrega('servicio');
}

function agregaImpuesto() {
    return aggrega('impuesto');    
}

function aggrega(what) {
    // here the code...
}

This is just an example. You could choose to pass all the different ids, classes as parameters.

Advice 4

You should wrap your all your code inside a function, to have your own private name space, and avoid to have conflicts with other code.

This is not strictly necessary in your case, but it is a good practice to use a module pattern, so you could reuse your code in different places.

Advice 5

You should choose a name convention and just follow.

I saw you use camel case, and even underscore:

• calcularImpuestos()

• genera_tabla(id)