HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Managing debit/credit accounts

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
creditdebitmanagingaccounts

Problem

I just want to know if there room for improvement or something I should redo to be better.

```
var dataCell = new Array();
dataCell[0] = document.getElementById("data0");
dataCell[1] = document.getElementById("data1");
dataCell[2] = document.getElementById("data2");
dataCell[3] = document.getElementById("data3");
dataCell[4] = document.getElementById("data4");
dataCell[5] = document.getElementById("data5");
dataCell[6] = document.getElementById("data6");
dataCell[7] = document.getElementById("data7");
dataCell[8] = document.getElementById("data8");
dataCell[9] = document.getElementById("data9");
var d = new Date();
var transactionNumber;
var post;

var accountId = new Array();
var account = new Array();
var credit = new Array();
var debit = new Array();

function newEntry() {
post = 0;
requestHighTransactionNumber();

dataCell[0].innerHTML = ""+d.getFullYear()+"-"+parseInt(d.getMonth()+1)+"-"+d.getDate()+"    0.000.00"+transactionNumber+"";
dataCell[1].innerHTML = "";
dataCell[2].innerHTML = "";
dataCell[3].innerHTML = "";
dataCell[4].innerHTML = "";
dataCell[5].innerHTML = "";
dataCell[6].innerHTML = "";
dataCell[7].innerHTML = "";
dataCell[8].innerHTML = "";
dataCell[9].innerHTML = "";
}

function addRow(offset) {
var i = offset;

dataCell[i].innerHTML = "    0.000.00"+transactionNumber+"";
post = i;

}

function genPost() {
var ajaxRequest; // The variable that makes Ajax possible!

try{
// Opera 8.0+, Firefox, Safari
ajaxRequest = new XMLHttpRequest();
} catch (e){
// Internet Explorer Browsers
try{
ajaxRequest = new ActiveXObject("Msxml2.XMLHTTP");
} catch (e) {
try{
ajaxRequest = new ActiveXObject("Microsoft.XMLHTTP");
} catch (e){
// Something went wrong
alert("Your

Solution

-
Where you have a load of html elements with similar functions it's useful to either include them in one containing element or give them all the same class. Then you can access them in the JavaScript using:

var dataCell = document.getElementsByClassName('dataCells');


or

var dataCell = document.getElementById('dataContainer').children;


-
You could use for loops to avoid repetition:

for (var i = 1; i < 9; i++) {
    dataCell[i] = '';
}


-
You use a while loop (while (i

-
You could use an Ajax function to avoid repetition of that bit of code:

function getAjaxRequest() {
    var ajaxRequest;
    try {
        // ...
        return ajaxRequest;
}


If you extend your code you will ultimately want a more capable function for controlling ajax requests, in which case look at this Stack Overflow answer. Not using synchronous requests is probably good advice. Also, I think the usual way of dealing with IE is as shown here.

-
It is better to build HTML using
createElement and appendChild rather than setting the innerHTML. In particular, using innerHTML` to set event handlers will get confusing and unreadable very quickly for code of any complexity. Better to do something like...

var creditTd = document.createElement('td');
creditTd.className = 'credit';
creditTd.id = 'credit0';
creditTd.addEventListener('keypress', function() {
    addRow(1);
});
creditTd.contentEditable = "true";
dataCell[0].appendChild(creditTd);


... even though this admittedly takes up a lot more lines. But you can then set up functions to remove any repetitive parts of this code.

Code Snippets

var dataCell = document.getElementsByClassName('dataCells');
var dataCell = document.getElementById('dataContainer').children;
for (var i = 1; i < 9; i++) {
    dataCell[i] = '';
}
for (var i = 0; i < post; i++) {...
function getAjaxRequest() {
    var ajaxRequest;
    try {
        // ...
        return ajaxRequest;
}

Context

StackExchange Code Review Q#27368, answer score: 6

Revisions (0)

No revisions yet.