7
\$\begingroup\$

Purpose

First, let me explain the title: I previously had managed my finances like an old fashioned company might, and it worked wonders for making financial decisions (at least for me). I wanted my friends and family to try but it took too much explaining of Excel to show them how. I thought now a better time than ever to learn web development, and thus this project was born.

This is my second attempt at it; the site is kind of basic-looking, as I have yet to learn CSS well, and in my first attempt I learned ugly CSS is hard to clean up without starting over. But it functions well, and I want another perspective on how my code is before I make the project any bigger.

Design/Functionality

Here's the basic function of the site: there's a dropdown menu through which users can choose to login and sign up if they like. By using the user login system, their data is saved, but they can still use the functionality fine without logging in or signing up. Signing up auto-logs a user in, and once logged in the Logout button replaces the Login/Signup button.

Users can add pseudo-accounts with their own balances (credit/debit) and they can also add transactions which automatically log debits and credits to accounts of their choosing. Certain actions are checked for and stopped, some for technical reasons and some for being nonsensical, but most silly actions are left to the user's discretion to avoid. Transactions and accounts can be removed as well, but the accounts must not be in any transactions in the list.

Basic Structure of Design

To make it loads easier, I'll describe the basic design flow of the code. the two main "lists" of accounts and tranasctions are in HTML tables. The Login/Signup, Logout, and Add Transaction/Account screens are all modal pop-ups. Data changes are done directly to the Firebase database, and then UI changes are made when the database state is changed. In the case of a logged out user, changes are just directly filled into the tables.

The database structure is as follows:

topLevel
    username (the first part of the email)
        accounts
            objectKey {
                name: NewAccount
                startingAmount: "$0.00"
                amountType: "Debit"
                description: "No description"
            }
        transactions
            objectKey {
                date: "1/1/2000"
                accounts: "<p>Account1</p>..."
                debits: "<p>'$0.00'</p>..."
                credits: "<p>'$0.00'</p>..."
                description: "No description"
            }

What I'm Looking For From This

I obviously don't expect a detailed, in-depth commentary on every part of my code (although I wouldn't object to one), as I acknowledge there is quite a lot of code and you almost certainly have no real reason to put that much time into my own personal project. So what I'd like out of this is very general, broad advice/notes which don't require a huge analysis:

  • Best Practices - I've taught myself everything I know about web development, and so I'm not fully familiar with conventions and best practices of the field. Maning conventions, spacing, variable/function choices, etc. - any aspects of my general code style that I can improve or update.
  • Execution of Ideas - I've probably tackled some problems (most likely on the JS side of things) in a very strange or convoluded way, as I'm not fully familiar with the whole language and its features. Especially Promises and Firebase's data retrieval; I'm still a little uneasy on the whole asynchronous code thing, so note if I'm not using or misusing async-specific functions, concepts, etc..
  • Blaring Red Flags - While this kind of ties in with the last two, anything I do which is a big no-no in the web development world.
  • Compatibility/Bugs - I tested every scenario I could think of for buggy behavior, and I tried my best to make it IE compatible (disgruntled sigh). While I don't want anyone doing my duty and compatibility testing for me, if you run into any problems or compatibility issues in the process just let me know of them. If you find any, I'll fix them in an edit on this post and on the hosting site below, and I'll keep everything functional as intended.
  • Anything Else - I'm not picky, guys. Any and every review of any part of my code will help me, so whatever you feel like doing I appreciate.

Code and Supplementary Info

Since the code snippets here won't seem to let me connect to Firebase, I've hosted the site on its end destination at 000webhost, here.

If you don't want to use your email and password, there's no verification so here's some fake login info for you that works and is already signed up:

codereviewemail@website.com
password

And here, at last, is the code:

HTML

<html>
<head>
  <title>Financial Manager v0.4</title>

  <link href="index.css" rel="stylesheet" type="text/css"/>

  <script src="https://www.gstatic.com/firebasejs/4.1.3/firebase.js"></script>
  <script>
    // Initialize Firebase
    var config = {
      apiKey: "AIzaSyAuJmoxZu0_A_vPMRxCwQ2Y8lv_kfKGsWY",
      authDomain: "financialmanager-3220c.firebaseapp.com",
      databaseURL: "https://financialmanager-3220c.firebaseio.com",
      projectId: "financialmanager-3220c",
      storageBucket: "",
      messagingSenderId: "68434361135"
    };
    firebase.initializeApp(config);
  </script>
</head>
<body>

  <div id="MenuBar">
    <button id="DropDownButton">Menu</button>
    <div id="DropDownMenu">
      <button id="LoginMenuButton" class="DropDownOption">Login/Sign Up</button>
    </div>
  </div>

  <div>
    <button id="AddTransaction">Add Transaction</button>

    <table id="TransactionTable">
      <thead>
        <th></th>
        <th>Date</th>
        <th>Account</th>
        <th>Debit</th>
        <th>Credit</th>
        <th>Description</th>
      </thead>
      <tbody id="TransactionTableBody">
      </tbody>
    </table>
  </div>

  <div>
    <button id="AddAccount">Add Account</button>

    <table id="AccountTable">
      <thead>
        <th></th>
        <th>Account</th>
        <th>Debit</th>
        <th>Credit</th>
        <th>Description</th>
      </thead>
      <tbody id="AccountTableBody">
      </tbody>
    </table>
  </div>

  <div id="LoginModalBackground" class="modalTop">
    <div id="LoginModal" class="modal">
      <button id="LoginClose" class="closeButton"></button>

      <input id="Email" placeholder="Email" class="LoginField" maxlength=128></input>
      <input id="Password" placeholder="Password" class="LoginField" maxlength=128></input>
      <div id="LoginButtonContainer">
        <button id="Login" class="LoginActionButton">Login</button>
        <button id="Sign Up" class="LoginActionButton">Sign Up</button>
      </div>
    </div>
  </div>

  <div id='AddTransactionModalBackground' class="modalTop">
    <div id='AddTransactionModal' class="modal">
      <button id="AddTransactionClose" class="closeButton"></button>

      <div class="LabeledInputContainer"><p>Date:        </p><input id="AddTransactionDateInput"></input></div>
      <div class="LabeledInputContainer"><p>Description: </p><input id="AddTransactionDescriptionInput" value="Lorem Ipsum..." maxlength=512></input></div>

      <table id="AddTransactionTable">
        <thead>
          <th><button id="AddTransactionAddAccount"></button></th>
          <th>Account</th>
          <th>Debit</th>
          <th>Credit</th>
        </thead>
        <tbody id="AddTransactionTableBody">
          <tr>
            <td></td>
            <td><select class='accountLists'></select></td>
            <td><input placeholder="0.00" class="resettable transactionMoneyField" maxlength=16></input></td>
            <td><input placeholder="0.00" class="resettable transactionMoneyField" maxlength=16></input></td>
          </tr>
          <tr>
            <td></td>
            <td><select class='accountLists'></select></td>
            <td><input placeholder="0.00" class="resettable transactionMoneyField" maxlength=16></input></td>
            <td><input placeholder="0.00" class="resettable transactionMoneyField" maxlength=16></input></td>
          </tr>
        </tbody>
      </table>

      <button id="AddTransactionSubmit">Submit Transaction</button>
    </div>
  </div>

  <div id='AddAccountModalBackground' class="modalTop">
    <div id='AddAccountModal' class="modal">
      <button id="AddAccountClose" class="closeButton"></button>

      <div class="LabeledInputContainer"><p>Name: </p><input id="AddAccountNameInput" placeholder="New Account" maxlength=64></input></div>
      <div class="LabeledInputContainer"><p>Description: </p><input id="AddAccountDescriptionInput" placeholder="Lorem Ipsum..." maxlength=512></input></div>
      <div class="LabeledInputContainer"><p>Starting Amount: </p><input id="AddAccountAmountInput" placeholder="0.00" maxlength=16></input></div>
      <div class="LabeledInputContainer">
        <p>Amount Type: </p>
        <select id="AddAccountTypeInput">
          <option value="Debit">Debit</option>
          <option value="Credit">Credit</option>
        <select>
      </div>

      <button id="AddAccountSubmit">Submit Transaction</button>
    </div>
  </div>

  <script src="onClickManager.js"></script>
</body>
<html>

Javascript

var loggedIn = false;
var currentUsername = "";

/*
Operations to perform on money formats
format() takes a decimal or [dollars, cents] and formats as "$XXXX.XX"
deformat() takes "$XXXX.XX" and returns [dollars, cents]
the other three take money in the form [dollars, cents] and return the result of their respective operations
*/
var format = function(input) {
  if(typeof input == "string") {
    if(input.match(/(\$\d+|\d+)\.\d{2}/g) && input==input.match(/(\$\d+|\d+)\.\d{2}/g)) {
      return "$"+input.match(/([1-9]\d*|0)\.\d{2}/g)[0];
    } else if(input.match(/(\$\d+|\d+)/g) && input == input.match(/(\$\d+|\d+)/g)[0]) {
      return "$"+input.match(/([1-9]\d*|0)/g)[0] + ".00";
    } else {
      return "false";
    }
  } else {
    if(input[1]==0) {
      return "$"+input[0].toString()+".00";
    } else if(input[1]<10) {
      return "$"+input[0].toString()+".0"+input[1].toString();
    } else {
      return "$"+input[0].toString()+"."+input[1].toString();
    }
  }
}
var deformat = function(string) {
  let dollars = string.match(/\$\d+/g)[0].substring(1);
  let cents = string.match(/\.\d{2}/g)[0].substring(1);
  return [parseInt(dollars), parseInt(cents)];
}
var addMoney = function(money1, money2) {
  let centCarry = Math.floor((money1[1]+money2[1])/100);
  return [money1[0]+money2[0]+centCarry, (money1[1]+money2[1])%100];
}
var subtractMoney = function(money1, money2) {
  let centCarry = 0;
  if(money1[1]-money2[1]<0) {
    centCarry = -1;
  }
  return [money1[0]-money2[0]+centCarry, (money1[1]-money2[1])%100];
}
var moneyGreaterThan = function(money1, money2) {
  return (money1[0]>money2[0]) || (money1[0]==money2[0] && money1[1]>money2[1]);
}

//Just to simplify some code and keep ugly regex hidden
var getUsername = function() {
  return firebase.auth().currentUser.email.match(/^[^@]+/g);
}

//grabs an account with the specified name from the accountTable, used for offline users
var getAccount = function(name) {
  let accountList = document.getElementById('AccountTableBody').children;
  for(let i = 0; i < accountList.length; i++) {
    if(accountList[i].cells[1].innerHTML == name) {
      return accountList[i];
    }
  }
  return "false";
}

//render takes the data and fills it into the HTML tables, add puts it in firebase
var renderAccount = function(name, startingAmount, amountType, desc) {
  let tableBody = document.getElementById('AccountTableBody');

  tableBody.insertRow(0);
  tableBody.rows[0].insertCell(0);
  tableBody.rows[0].insertCell(1);
  tableBody.rows[0].insertCell(2);
  tableBody.rows[0].insertCell(3);
  tableBody.rows[0].insertCell(4);

  let removeButton = document.createElement('button');
  removeButton.className = "removalButton";
  removeButton.onclick = function() {
    var thisRow = this.parentNode.parentNode;
    if(loggedIn) {
      let canDelete = true;
      firebase.database().ref(currentUsername + "/transactions").once('value').then(function(dataSnapshot) {

        if(dataSnapshot.val() == null || dataSnapshot.val().length == 0) {
          return;
        }

        let data = dataSnapshot.val();
        let keys = Object.keys(data);

        for(let i = 0; i < keys.length; i++) {
          if(data[keys[i]].accounts.search(name) > -1) {
            canDelete = false;
          }
        }
      }).then(function() {
        if(canDelete) {
          firebase.database().ref(currentUsername + "/accounts").once('value').then(function(dataSnapshot) {
            let data = dataSnapshot.val();
            let keys = Object.keys(data);

            for(let i = 0; i < keys.length; i++) {
              //since no two accounts can have the same name, this will give us the desired account
              if(data[keys[i]].name == name) {
                firebase.database().ref(currentUsername + "/accounts/" + keys[i]).remove();
              }
              break;
            }
          });
        } else {
          alert("Cannot delete account, transaction history with this account exists.");
        }
      });
    } else {
      let transactionTableBody = document.getElementById("TransactionTableBody");

      for(let row = 0; row < transactionTableBody.rows.length; row++) {
        if(transactionTableBody.rows[row].cells[2].innerHTML.search(name) > -1) {
          alert("Cannot delete account, transaction history with this account exists.");
          return;
        }
      }

      thisRow.parentNode.removeChild(thisRow);
    }
  }
  tableBody.rows[0].cells[0].appendChild(removeButton);

  tableBody.rows[0].cells[1].innerHTML = name;
  if(amountType == "Debit") {
    tableBody.rows[0].cells[2].innerHTML = startingAmount;
    tableBody.rows[0].cells[3].innerHTML = "$0.00";
  } else {
    tableBody.rows[0].cells[2].innerHTML = "$0.00";
    tableBody.rows[0].cells[3].innerHTML = startingAmount;
  }
  tableBody.rows[0].cells[4].innerHTML = desc;
}
var addAccount = function(name, startingAmount, amountType, desc) {

  if(firebase.auth().currentUser) {
    firebase.database().ref(getUsername() + "/accounts").push({name: name, startingAmount: startingAmount, amountType: amountType, description: desc})
  }

}
var renderTransaction = function(date, accounts, debits, credits, description) {
  let otherTableBody = document.getElementById('TransactionTableBody');

  otherTableBody.insertRow(0);
  otherTableBody.rows[0].insertCell(0);
  otherTableBody.rows[0].insertCell(1);
  otherTableBody.rows[0].insertCell(2);
  otherTableBody.rows[0].insertCell(3);
  otherTableBody.rows[0].insertCell(4);
  otherTableBody.rows[0].insertCell(5);

  let removeButton = document.createElement('button');
  removeButton.className = "removalButton";
  removeButton.onclick = function() {
    let thisRow = this.parentNode.parentNode;
    if(loggedIn) {
      firebase.database().ref(currentUsername + "/transactions").once('value').then(function(dataSnapshot) {
        let data = dataSnapshot.val();
        let keys = Object.keys(data);

        for(let i = 0; i < keys.length; i++) {
          //if every field is the same, this is the transaction we're looking for
          if(data[keys[i]].date == date && data[keys[i]].accounts == accounts && data[keys[i]].debits == debits && data[keys[i]].credits == credits && data[keys[i]].description == description) {
            firebase.database().ref(currentUsername + "/transactions/" + keys[i]).remove();
          }
          break;
        }
      });
    } else {
      thisRow.parentNode.removeChild(thisRow);
    }
  }
  otherTableBody.rows[0].cells[0].appendChild(removeButton);

  otherTableBody.rows[0].cells[1].innerHTML = date;
  otherTableBody.rows[0].cells[2].innerHTML = accounts;
  otherTableBody.rows[0].cells[3].innerHTML = debits;
  otherTableBody.rows[0].cells[4].innerHTML = credits;
  otherTableBody.rows[0].cells[5].innerHTML = description;
}
var addTransaction = function(date, accounts, debits, credits, description) {

  if(firebase.auth().currentUser) {
    firebase.database().ref(getUsername() + "/transactions").push({date: date, accounts: accounts, debits: debits, credits: credits, description: description})
  }
}

//clears 1 or both tables
var clearTables = function() {
  clearTable("accounts");
  clearTable("transactions");
}
var clearTable = function(tableType) {
  if(tableType == "accounts") {
    let rows1 = document.getElementById("AccountTableBody").getElementsByTagName("tr");
    for(let i = rows1.length-1; i >= 0; i--) {
      rows1[i].remove();
    }
  } else if(tableType == "transactions") {
    let rows2 = document.getElementById("TransactionTableBody").getElementsByTagName("tr");
    for(let i = rows2.length-1; i >= 0; i--) {
      rows2[i].remove();
    }
  }
}

//these are the functions to reload data when it changes in firebase
var loadAccountData = function(dataSnapshot) {

  if(dataSnapshot.val() == null || dataSnapshot.val().length == 0) {
    clearTable("accounts");
    return;
  }
  let data = []
  let keys = Object.keys(dataSnapshot.val());

  for(let i = 0; i < keys.length; i++) {
    data.push(dataSnapshot.val()[keys[i]]);
  }

  clearTable("accounts");
  for(let i = 0; i < data.length; i++) {
    renderAccount(data[i].name,data[i].startingAmount,data[i].amountType,data[i].description);
  }
}
var loadTransactionData = function(dataSnapshot) {
  if(dataSnapshot.val() == null || dataSnapshot.val().length == 0) {
    clearTable('transactions');
    return;
  }
  let data = []
  let keys = Object.keys(dataSnapshot.val());

  for(let i = 0; i < keys.length; i++) {
    data.push(dataSnapshot.val()[keys[i]]);
  }

  clearTable("transactions");
  for(let i = 0; i < data.length; i++) {
    renderTransaction(data[i].date,data[i].accounts,data[i].debits,data[i].credits, data[i].description);
  }
}

//onclick event handlers for the drop down menu
document.getElementById('DropDownButton').onclick = function() {
  let curDisplayMode = document.getElementById('DropDownMenu').style.display;
  document.getElementById('DropDownMenu').style.display = curDisplayMode == "flex" ? "none" : "flex";
}
document.getElementById('LoginMenuButton').onclick = function() {
  if(!loggedIn) {
    document.getElementById('LoginModalBackground').style.display = "flex";
  } else {
    firebase.auth().signOut();
    clearTables();
  }
  document.getElementById("DropDownMenu").style.display = "none";
}
document.getElementById("LoginClose").onclick = function() {
  document.getElementById("LoginModalBackground").style.display = "none";
}
var checkLoginInfoValidity = function(email, password) {
  if(email.length == 0) {
    return "Please enter a valid email address!";
  }
  if(password.length <= 6) {
    return "Password must be longer than 6 characters!"
  }
  return "valid";
}
document.getElementById("Login").onclick = function() {

  let email = document.getElementById("Email").value;
  let password = document.getElementById("Password").value;

  let message = checkLoginInfoValidity(email, password);
  if(message != "valid") {
    alert(message);
    return;
  }

  const promise = firebase.auth().signInWithEmailAndPassword(email, password);
  promise.catch(e => alert(e));

  document.getElementById("LoginModalBackground").style.display = "none";
}
document.getElementById("Sign Up").onclick = function() {

  let email = document.getElementById("Email").value;
  let password = document.getElementById("Password").value;

  let message = checkLoginInfoValidity(email, password);
  if(message != "valid") {
    alert(message);
    return;
  }

  const promise = firebase.auth().createUserWithEmailAndPassword(email, password);
  promise.catch(e => alert(e));

  document.getElementById("LoginModalBackground").style.display = "none";
}

//onclick event handlers for the add transaction menu
document.getElementById('AddTransaction').onclick = function() {
  if(document.getElementById('AccountTableBody').rows.length < 2) {
    alert("There must be two or more accounts to process a transaction!!");
    return;
  }

  let accountLists = document.getElementsByClassName('accountLists');
  let listOfAccountNames = [];
  for(let i = 0; i < document.getElementById('AccountTableBody').rows.length; i++) {
    listOfAccountNames.push(document.getElementById('AccountTableBody').rows[i].cells[1].innerHTML);
  }
  for(let i = 0; i < accountLists.length; i++) {
    let newOption = document.createElement('option');
    newOption.text = "Please Select an Account";
    newOption.value = "invalid";
    accountLists[i].add(newOption);

    let divider = document.createElement('option');
    divider.text = "--------------------";
    divider.disabled = true;
    accountLists[i].add(divider);

    for(let j = 0; j < listOfAccountNames.length; j++) {
      let newOption = document.createElement('option');
      newOption.text = listOfAccountNames[j];
      newOption.value = listOfAccountNames[j];
      accountLists[i].add(newOption);
    }
  }

  document.getElementById('AddTransactionModalBackground').style.display = "flex";
  let d = new Date();
  document.getElementById('AddTransactionDateInput').value = (d.getMonth()+1) + "/" + d.getDate() + "/" + d.getFullYear();
}
document.getElementById('AddTransactionClose').onclick = function() {
  document.getElementById('AddTransactionModalBackground').style.display = "none";
}
document.getElementById('AddTransactionAddAccount').onclick = function() {
  let tableBody = document.getElementById('AddTransactionTableBody');
  tableBody.insertRow(0);
  let row = tableBody.rows[0];
  row.insertCell(0);
  row.insertCell(1);
  row.insertCell(2);
  row.insertCell(3);
  row.cells[1].outerHTML = "<td><input></input></td>";
  row.cells[2].outerHTML = "<td><input></input></td>";
  row.cells[3].outerHTML = "<td><input></input></td>";
}
var checkTransactionValidity = function(tableBody) {
  for(let row = 0; row < tableBody.rows.length; row++) {
    let options = tableBody.rows[row].cells[1].children[0].options;
    if(options[options.selectedIndex].value == "invalid") {
      return "Please select only valid accounts."
    }
  }

  let fields = document.getElementsByClassName('transactionMoneyField');
  for(let i = 0; i < fields.length; i++) {
    if(format(fields[i].value)!="false") {
      fields[i].value = format(fields[i].value);
    } else {
      return "Invalid Money Amount!!";
    }
  }

  return "valid"
}
document.getElementById('AddTransactionSubmit').onclick = function() {
  let tableBody = document.getElementById('AddTransactionTableBody');

  let message = checkTransactionValidity(tableBody);
  if(message != "valid") {
    alert(message);
    return;
  }

  if(loggedIn) {

    firebase.database().ref(currentUsername + "/accounts").once('value').then(function(dataSnapshot) {
      let data = dataSnapshot.val();
      let keys = Object.keys(data);
      for(let i = 0; i < tableBody.rows.length; i++) {
        let accountMod = tableBody.rows[i]
        let accountModName = accountMod.cells[1].children[0].options[accountMod.cells[1].children[0].selectedIndex].value;

        for(let j = 0; j < keys.length; j++) {
          if(data[keys[j]].name == accountModName) {

            let account = data[keys[j]];

            let newDebit = [0,0];
            let newCredit = [0,0];
            if(account.amountType == "Debit") {
              newDebit = addMoney(deformat(account.startingAmount), deformat(format(accountMod.cells[2].children[0].value)));
              newCredit = addMoney(deformat("$0.00"), deformat(format(accountMod.cells[3].children[0].value)));
            } else {
              newDebit = addMoney(deformat("$0.00"), deformat(format(accountMod.cells[2].children[0].value)));
              newCredit = addMoney(deformat(account.startingAmount), deformat(format(accountMod.cells[3].children[0].value)));
            }

            //Normalize
            if(moneyGreaterThan(newDebit, newCredit)) {
              newDebit = subtractMoney(newDebit, newCredit);
              firebase.database().ref(currentUsername + "/accounts/" + keys[j] + "/startingAmount").set(format(newDebit));
              firebase.database().ref(currentUsername + "/accounts/" + keys[j] + "/amountType").set("Debit");
            } else {
              newCredit = subtractMoney(newCredit, newDebit);
              firebase.database().ref(currentUsername + "/accounts/" + keys[j] + "/startingAmount").set(format(newCredit));
              firebase.database().ref(currentUsername + "/accounts/" + keys[j] + "/amountType").set("Credit");
            }
            break;
          }
        }
      }
    }).then(function() {
      let date = document.getElementById('AddTransactionDateInput').value;
      let descriptionInput = document.getElementById('AddTransactionDescriptionInput');
      let description = descriptionInput.value;

      let accounts = "";
      let debits = "";
      let credits = "";

      for(let i = 0; i < tableBody.rows.length; i++) {
          accounts += "<p>"+tableBody.rows[i].cells[1].children[0].value+"</p>";
          debits += "<p>"+tableBody.rows[i].cells[2].children[0].value+"</p>";
          credits += "<p>"+tableBody.rows[i].cells[3].children[0].value+"</p>";
      }

      addTransaction(date, accounts, debits, credits, description);

      document.getElementById('AddTransactionDescriptionInput').value = descriptionInput.defaultValue;
      for(let i = 0; i < document.getElementsByClassName('resettable').length; i++) {
        document.getElementsByClassName('resettable')[i].value = document.getElementsByClassName('resettable')[i].defaultValue;
      }
    });

  } else {
    for(let i = 0; i < tableBody.rows.length; i++) {
      let account = getAccount(tableBody.rows[i].cells[1].children[0].value);
      let accountMod = tableBody.rows[i];

      let accountValues = [account.cells[2].innerHTML,account.cells[3].innerHTML];
      let accountModValues = [accountMod.cells[2].children[0].value,accountMod.cells[3].children[0].value];

      //Debit
      let newAmount1 = addMoney(deformat(accountValues[0]), deformat(accountModValues[0]));
      accountValues[0] = format(newAmount1);

      //Credit
      let newAmount2 = addMoney(deformat(accountValues[1]), deformat(accountModValues[1]));
      accountValues[1] = format(newAmount2);

      //Normalize (all debit or all credit)
      if(accountValues[0] != "$0.00" && accountValues[1] != "$0.00") {
        if(moneyGreaterThan(deformat(accountValues[0]),deformat(accountValues[1]))) {
          account.cells[2].innerHTML = format(subtractMoney(deformat(accountValues[0]),deformat(accountValues[1])));
          account.cells[3].innerHTML = format([0,0]);
        } else {
          account.cells[2].innerHTML = format([0,0]);
          account.cells[3].innerHTML = format(subtractMoney(deformat(accountValues[1]),deformat(accountValues[0])));
        }
      }
    }

    let date = document.getElementById('AddTransactionDateInput').value;
    let descriptionInput = document.getElementById('AddTransactionDescriptionInput');
    let description = descriptionInput.value;

    let accounts = "";
    let debits = "";
    let credits = "";

    for(let i = 0; i < tableBody.rows.length; i++) {
        accounts += "<p>"+tableBody.rows[i].cells[1].children[0].value+"</p>";
        debits += "<p>"+tableBody.rows[i].cells[2].children[0].value+"</p>";
        credits += "<p>"+tableBody.rows[i].cells[3].children[0].value+"</p>";
    }

    renderTransaction(date, accounts, debits, credits, description);

    document.getElementById('AddTransactionDescriptionInput').value = descriptionInput.defaultValue;
    for(let i = 0; i < document.getElementsByClassName('resettable').length; i++) {
      document.getElementsByClassName('resettable')[i].value = document.getElementsByClassName('resettable')[i].defaultValue;
    }
  }

  document.getElementById('AddTransactionModalBackground').style.display = "none";

}

//onclick handlers for the add account menu
document.getElementById('AddAccount').onclick = function() {
  document.getElementById('AddAccountModalBackground').style.display = "flex";
}
document.getElementById('AddAccountClose').onclick = function() {
  document.getElementById('AddAccountModalBackground').style.display = "none";
}
var checkAccountValidity = function(name, description, startingAmount, type) {

  let tableBody = document.getElementById('AccountTableBody');
  for(let i = 0; i < tableBody.rows.length; i++) {
    if(name == tableBody.rows[i].cells[1].innerHTML) {
      return "Account with that name already exists!";
    }
  }

  if(startingAmount != 0 && type != "Debit" && type != "Credit") {
    return "Please select a starting amount type";
  }

  return "valid";
}
document.getElementById('AddAccountSubmit').onclick = function() {
  let tableBody = document.getElementById('AccountTableBody');

  let nameInput = document.getElementById('AddAccountNameInput');
  let descriptionInput = document.getElementById('AddAccountDescriptionInput');
  let startingAmountInput = document.getElementById('AddAccountAmountInput');
  let amountTypeInput = document.getElementById('AddAccountTypeInput');

  let name = nameInput.value;
  let description = descriptionInput.value;
  let startingAmount = startingAmountInput.value;
  let amountType = amountTypeInput.value;

  if(format(startingAmount)!="false") {
    startingAmount = format(startingAmount);
  } else {
    alert("Invalid Starting Amount!!");
    return;
  }

  let message = checkAccountValidity(name, description, startingAmount, amountType);
  if(message != "valid") {
    alert(message);
    return;
  }

  if(loggedIn) {
    addAccount(name, startingAmount, amountType, description);
  } else {
    renderAccount(name, startingAmount, amountType, description);
  }

  document.getElementById('AddAccountNameInput').value = nameInput.defaultValue;
  document.getElementById('AddAccountDescriptionInput').value = descriptionInput.defaultValue;
  document.getElementById('AddAccountAmountInput').value = startingAmountInput.defaultValue;
  document.getElementById('AddAccountTypeInput').getElementsByTagName('option')[0].selected = "selected";

  document.getElementById('AddAccountModalBackground').style.display = "none";

}

//called when user login state changes
firebase.auth().onAuthStateChanged(firebaseUser => {
  if(firebaseUser) {

    loggedIn = true;
    document.getElementById("LoginMenuButton").innerHTML = "Logout";

    currentUsername = getUsername();

    clearTables();
    firebase.database().ref(currentUsername + "/accounts").on("value", loadAccountData);
    firebase.database().ref(currentUsername + "/transactions").on("value", loadTransactionData);
  } else {

    loggedIn = false;
    document.getElementById("LoginMenuButton").innerHTML = "Login/Sign Up";

    clearTables();
    firebase.database().ref(currentUsername + "/accounts").off("value", loadAccountData);
    firebase.database().ref(currentUsername + "/transactions").off("value", loadTransactionData);

    currentUsername = "";
  }
})

CSS

body {
  display: flex;
  flex-direction: row;
  justify-content: center;
}


#DropDownButton {
  position: absolute;
  left: 0;
  top: 0;

  width: 150px;
  height: 50px;

  border: none;

  background-color: rgb(180,180,180);
}
#DropDownMenu {
  display: none;
  flex-direction: column;
  justify-content: flex-start;
  align-items: flex-start;

  position: absolute;
  left: 0;
  top: 50px;
}
.dropDownOption {
  width: 150px;
  height: 50px;

  border: none;
}


#LoginModal {
  display: flex;
  flex-direction: column;
  justify-content: center;
}
.LoginField {
  margin: 5px 10px 5px 10px;
}
#LoginButtonContainer {
  display: flex;
  flex-direction: row;
}
.LoginActionButton {
  margin: 5px 0px 5px 10px;
}


.tableCell {
  display: flex;
  flex-direction: column;
  justify-content: flex-start;
}



.modalTop {
  display: none;

  width: 100%;
  height: 100%;

  position: fixed;
  left: 0;
  top: 0;

  align-items: center;
  justify-content: center;

  background-color: rgba(0,0,0,0.6);
}
.modal {
  position: relative;

  width: 50%;
  height: 50%;

  min-width: 400px;
  min-height: 400px;

  background-color: #fff;
}
.closeButton {
  width: 16px;
  height: 16px;

  position: absolute;
  top: 5px;
  right: 5px;

  background-image: url("clearImage.png");

  border: none;
}

.removalButton {
  width: 16px;
  height: 16px;

  background-image: url('clearImage.png');

  border: none;
}

#AddTransactionAddAccount {
  width: 16px;
  height: 16px;

  background-image: url('addImage.png');

  border: none;
}
#AddTransactionSubmit {
  position: absolute;

  bottom: 5px;
  right: 5px;
}


.labeledInputContainer {
  display: flex;
  flex-direction: row;
  align-items: baseline;
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Welcome to CodeReview, this looks like a very well written question, I'm sure you'll get some answers \$\endgroup\$ – IEatBagels Aug 10 '17 at 4:47
2
\$\begingroup\$

I know it has been more than two years since you posted this question so I apologize that nobody has supplied an answer yet. I also realize that you likely have learned a few things since then and I apologize if anything below is old news to you.

My overall take on this code is that it is decent for a self-taught JS programmer. However there are multiple inefficiencies and other areas that could be improved.

The biggest concern I have with this code is the possibility for memory leaks because some DOM elements have onclick callbacks set to anonymous functions. This could lead to memory leaks if those elements are removed from the DOM (refer to this SO post for more details on that topic). A better technique would be to use event delegation to how to handle events based on the target.


I see the let keyword used to declare many local variables. This is good but for any value that shouldn't be re-assigned, const can be used. This helps avoid accidental re-assignment.


In this block there are three DOM queries per iteration (when there could only be one per page load):

for(let i = 0; i < document.getElementsByClassName('resettable').length; i++) {
  document.getElementsByClassName('resettable')[i].value = document.getElementsByClassName('resettable')[i].defaultValue;
}

document.getElementsByClassName() returns a live HTMLCollection, which means it doesn't need to be re-queried during the lifetime of the page. It could be stored once and even if the DOM is updated that collection will be updated.

let resettables;
document.addEventListener('DOMContentLoaded', function() {
  resettables = document.getElementsByClassName('resettable');
});

Then that block can utilize that variable:

for(let i = 0; i < resettables.length; i++) {
  resettables[i].value = resettables[i].defaultValue;
}

And if features are supported, then a for...of loop can also help simplify this:

for(const resettable of resettables) {
  resettable.value = resettable.defaultValue;
}

There are a few places with redundant blocks that could be simplified with a for loop - e.g.

tableBody.insertRow(0);
tableBody.rows[0].insertCell(0);
tableBody.rows[0].insertCell(1);
tableBody.rows[0].insertCell(2);
tableBody.rows[0].insertCell(3);
tableBody.rows[0].insertCell(4);
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thanks for the review - I had forgotten about this entirely, and now do a lot of computational stuff with python. Now I'll have to revisit this, and make some of the changes you recommended. \$\endgroup\$ – Vedvart1 Nov 20 '19 at 13:26
3
\$\begingroup\$

Another thing I wanted to touch on is the formatting functions. For example, the deformat() function.

var deformat = function(string) {
  let dollars = string.match(/\$\d+/g)[0].substring(1);
  let cents = string.match(/\.\d{2}/g)[0].substring(1);
  return [parseInt(dollars), parseInt(cents)];
}

It seems like this function expects the input string to start with a dollar sign, contain one or more digits, a period and then two digits. If the string doesn't match that format then there will most likely be an error because string.match() wouldn't return an array with at least one element (in both of the first two lines) and thus it wouldn't be able to call the substring function on undefined.

The format could be consolidated to a single regular expression that uses capturing groups as long as the g modifier is removed. And the Number constructor could be used to create numbers instead of using Number.parseInt().

const dollarCentsPattern = /\$(\d+)\.(\d{2})/;
var deformat = function(string) {
  if (!dollarCentsPattern.test(string)) {
    return []; //or throw an error?
  }
  const matches = string.match(dollarCentsPattern);
  if (matches.length < 3) {
    return []; //or throw an error?
  }
  return [Number(matches[1]), Number(matches[2])];
}

Named capturing groups could also be used - for example:

const dollarCentsPattern = /\$(?<dollars>\d+)\.(?<cents>\d{2})/;

Then if it matched, the return value from .match() would contain a groups object with dollars and cents for those matched values.

It should be noted that regular expressions should only be used when required as they can be expensive. This task could be solved simply by splitting the string on a period, expecting no more than two parts:

function deformat(string) {
  var parts = string.split('.', 2);
  if (parts.length < 2) {
    return []; //or throw error?
  }
  return [Number(parts[0].substring(1)), Number(parts[1].substring(0, 2))]; 
}
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thanks, this is especially helpful - I have some learning to do still with regex. \$\endgroup\$ – Vedvart1 Nov 21 '19 at 15:25
  • \$\begingroup\$ A lot to learn with regex :p I have been learning it for 15 years now and still surprised about how many new ways or things you can do in regex. Enjoy it though it’s fun \$\endgroup\$ – soulshined Nov 29 '19 at 11:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.