patternjavascriptMinor
Count calls and update table accordingly
Viewed 0 times
callsupdateandcountaccordinglytable
Problem
This is literally the first time I've ever used JavaScript so I know it's really bad and how to improve it just not how to implement it. Basically, I need to loop through all the elements instead of manually checking and updating each one. Any suggestions on how to do that, and other recommendations?
`
h1{
font-size: 2.5em;
font-family: Arial, Helvetica, sans-serif;
}
table {
border: 5px ridge black;
width:99%;
margin-left: .1%;
background-color: #bdbdbd;
}
#btnNextCall{
margin-left: .1%;
width: 99%;
height: 50px;
background-color: #58acfa;
border: 2px solid #58acfa;
color: white;
font-size: 2em;
font-family: "Arial", sans-serif;
transition-duration: .3s;
}
#btnNextCall:hover{
background-color: white;
color: black;
cursor: pointer;
}
th{
padding: 10px;
font-size: 1.6em;
background-color: #58acfa;
color: white;
text-shadow: 0 0 5px black;
font-family: "Arial", sans-serif;
}
td:first-child{
text-align: left;
}
td{
text-align: center;
padding: 5px;
font-size: 1.2em;
font-family: "Arial", sans-serif;
}
table tr:nth-child(odd) td{
background-color: #cdcdcd;
}
table tr:nth-child(even) td{
background-color: #e6e6e6;
}
input[type="checkbox"]{
width:20px;
height:20px;
cursor: pointer;
`
h1{
font-size: 2.5em;
font-family: Arial, Helvetica, sans-serif;
}
table {
border: 5px ridge black;
width:99%;
margin-left: .1%;
background-color: #bdbdbd;
}
#btnNextCall{
margin-left: .1%;
width: 99%;
height: 50px;
background-color: #58acfa;
border: 2px solid #58acfa;
color: white;
font-size: 2em;
font-family: "Arial", sans-serif;
transition-duration: .3s;
}
#btnNextCall:hover{
background-color: white;
color: black;
cursor: pointer;
}
th{
padding: 10px;
font-size: 1.6em;
background-color: #58acfa;
color: white;
text-shadow: 0 0 5px black;
font-family: "Arial", sans-serif;
}
td:first-child{
text-align: left;
}
td{
text-align: center;
padding: 5px;
font-size: 1.2em;
font-family: "Arial", sans-serif;
}
table tr:nth-child(odd) td{
background-color: #cdcdcd;
}
table tr:nth-child(even) td{
background-color: #e6e6e6;
}
input[type="checkbox"]{
width:20px;
height:20px;
cursor: pointer;
Solution
There are many commendable aspects of your mini application:
And most of all, if this is truly the first time you've used JavaScript, and you managed to get this working, I'm impressed!
Unfortunately, the HTML and the JavaScript are both heavily repetitive. In my opinion, this approach will end up being a maintenance headache, if you ever need to add items to or remove items from the rubric.
Furthermore, this problem is something I wouldn't even think of tackling using raw JavaScript. You would need some kind of abstraction layer. In my opinion, jQuery is almost essential.
For those reasons, I recommend starting with a fresh approach. (I don't mean to discourage you, but I think you'll see the advantage once you compare the code.)
General principles
If you have many of the same kind of variable, don't use multiple variables. Use an array instead.
Find a way to use the same click handler for multiple user interface elements. For example, you should only need to write one collapseCat()
'use strict';
// Initialize the metrics in each category to 0
// http://stackoverflow.com/q/1295584
var metricCounts = rubric.map(function(category) {
for (var categoryName in category) {
return new Array(category[categoryName].length).fill(0);
}
});
var callCount = 0;
// Create the HTML table body
for (var c = 0; c ')
.append($('')
.append($('')
.text(categoryName)
.append('Collapse')
)
);
$table.append($cat);
var metrics = category[categoryName];
for (var m = 0; m ')
.append($('').text(metrics[m]))
.append('')
.append('/')
.append('%');
$cat.append($row);
}
}
}
// Handler for collapse / expand
$table.on('click', 'button', function toggleCollapse() {
$(this).closest('tbody').toggleClass('collapse');
});
// Handler for "Next Call" button
$button.on('click', function nextCall() {
// Update metrics and clear the checkboxes
++callCount;
$table.find('input[type=checkbox]:checked').each(function() {
var headers = $(this).closest('td').attr('headers');
var match = /metric(\d+)\.(\d+)/.exec(headers);
++metricCounts[match[1]][match[2]];
}).prop('checked', false);
// Display new statistics
for (var c = 0; c
`table.tracker caption {
margin-bottom: 0.75em;
text-align: left;
font-size: 2.5em;
font-family: Arial, Helvetica, sans-serif;
font-weight: bold;
}
table.tracker {
border: 5px ridge black;
width: 99%;
margin-left: .1%;
background-color: #bdbdbd;
}
table.tracker > thead th {
padding: 10px;
font-size: 1.6em;
font-weight: bold;
background-color: #58acfa;
color: white;
text-shadow: 0 0 5px black;
font-family: "Arial", sans-serif;
}
table.tracker th,
table.tracker td {
text-align: center;
padding: 5px;
font-size: 1.2em;
font-weight: normal;
font-family: "Arial", sans-serif;
}
table.tracker > tbody > tr > th {
text-align: left;
}
table.tracker > tbody > tr:nth-child(even) {
background-color: #cdcdcd;
}
table.tracker > tbody > tr:nth-child(odd) {
background-color: #e6e6e6;
}
input[type="checkbox"] {
width: 20px;
height: 20px;
cursor: pointer;
}
table.tracker > tbody > tr:first-child > th {
border-top: 1px solid black;
font-size: 1.4em;
text-decoration: underline;
}
table.tracker > tbody.collapse > tr:nth-child(n+2) {
display: none;
}
button.tracker {
margin-left: .1%;
width: 99%;
height: 50px;
background-color: #58acfa;
border: 2px solid
- The HTML is beautifully laid out.
- You've used the HTML5 doctype, and the `
declaration.
- The CSS is readable, and the subtle animation effects are a nice touch.
And most of all, if this is truly the first time you've used JavaScript, and you managed to get this working, I'm impressed!
Unfortunately, the HTML and the JavaScript are both heavily repetitive. In my opinion, this approach will end up being a maintenance headache, if you ever need to add items to or remove items from the rubric.
Furthermore, this problem is something I wouldn't even think of tackling using raw JavaScript. You would need some kind of abstraction layer. In my opinion, jQuery is almost essential.
For those reasons, I recommend starting with a fresh approach. (I don't mean to discourage you, but I think you'll see the advantage once you compare the code.)
General principles
If you have many of the same kind of variable, don't use multiple variables. Use an array instead.
Find a way to use the same click handler for multiple user interface elements. For example, you should only need to write one collapseCat()
function; the function can decide which category to collapse by examining the source of the click event.
Keeping the HTML and JavaScript code consistent with each other is a pain. Ideally, you want to define the rubric just once. You could either have the JavaScript scrape the HTML, or have the JavaScript generate the HTML. Since the HTML table is verbose and repetitive, I'd rather define a JavaScript data structure and generate the HTML from it.
HTML tables have some markup features you could take advantage of:
- Label each
with a headers attribute, to make it easy to find out which row and column it is associated with.
- Use a
to mark the first row.
- Use a separate
for each category. (This messes up the strict zebra stripe alternation. With the proper header styling, I don't think that's a bad thing.)
- Optionally use a
instead of .
Suggested solution
function tracker($table, $button, rubric) {'use strict';
// Initialize the metrics in each category to 0
// http://stackoverflow.com/q/1295584
var metricCounts = rubric.map(function(category) {
for (var categoryName in category) {
return new Array(category[categoryName].length).fill(0);
}
});
var callCount = 0;
// Create the HTML table body
for (var c = 0; c ')
.append($('')
.append($('')
.text(categoryName)
.append('Collapse')
)
);
$table.append($cat);
var metrics = category[categoryName];
for (var m = 0; m ')
.append($('').text(metrics[m]))
.append('')
.append('/')
.append('%');
$cat.append($row);
}
}
}
// Handler for collapse / expand
$table.on('click', 'button', function toggleCollapse() {
$(this).closest('tbody').toggleClass('collapse');
});
// Handler for "Next Call" button
$button.on('click', function nextCall() {
// Update metrics and clear the checkboxes
++callCount;
$table.find('input[type=checkbox]:checked').each(function() {
var headers = $(this).closest('td').attr('headers');
var match = /metric(\d+)\.(\d+)/.exec(headers);
++metricCounts[match[1]][match[2]];
}).prop('checked', false);
// Display new statistics
for (var c = 0; c
`table.tracker caption {
margin-bottom: 0.75em;
text-align: left;
font-size: 2.5em;
font-family: Arial, Helvetica, sans-serif;
font-weight: bold;
}
table.tracker {
border: 5px ridge black;
width: 99%;
margin-left: .1%;
background-color: #bdbdbd;
}
table.tracker > thead th {
padding: 10px;
font-size: 1.6em;
font-weight: bold;
background-color: #58acfa;
color: white;
text-shadow: 0 0 5px black;
font-family: "Arial", sans-serif;
}
table.tracker th,
table.tracker td {
text-align: center;
padding: 5px;
font-size: 1.2em;
font-weight: normal;
font-family: "Arial", sans-serif;
}
table.tracker > tbody > tr > th {
text-align: left;
}
table.tracker > tbody > tr:nth-child(even) {
background-color: #cdcdcd;
}
table.tracker > tbody > tr:nth-child(odd) {
background-color: #e6e6e6;
}
input[type="checkbox"] {
width: 20px;
height: 20px;
cursor: pointer;
}
table.tracker > tbody > tr:first-child > th {
border-top: 1px solid black;
font-size: 1.4em;
text-decoration: underline;
}
table.tracker > tbody.collapse > tr:nth-child(n+2) {
display: none;
}
button.tracker {
margin-left: .1%;
width: 99%;
height: 50px;
background-color: #58acfa;
border: 2px solid
Context
StackExchange Code Review Q#123727, answer score: 3
Revisions (0)
No revisions yet.