Who's in the fellowship? When are Frodo, and the rest of the gang, together

As a variant on the fizzbuzz concept, and as an exercise for learning JavaScript, HTML5, and CSS (I know none of them very well).

The regular fizzbuzz is somewhat tired, but having a web-based, visualizable output seems to be useful, and being able to adjust the inputs at will, allows you to see the impact of how the modulo changes.



var fizzLoaded = false;
var fizzDiv, fizzFrom, fizzTo, fizzPlayers;

function fizzLoad() {
if (fizzLoaded) {
return;
}
fizzLoaded = true;
var fizzForm = document.getElementById('fizzbuzz');
fizzFrom = document.getElementById('rangeFrom');
fizzTo = document.getElementById('rangeTo');
fizzPlayers = [
document.getElementById('frodo'),
document.getElementById('sam'),
document.getElementById('merry'),
document.getElementById('pippin'),
document.getElementById('bilbo')
];
fizzDiv = document.getElementById('fizzOut');
}

function restrictRange() {
var rFrom = parseInt(fizzFrom.value);
var rTo = parseInt(fizzTo.value);
fizzTo.min = rFrom;
fizzFrom.max = rTo;
}

function validateValues() {
var rFrom = parseInt(fizzFrom.value);
var rTo = parseInt(fizzTo.value);
if (rTo 100) {
alert("Illegal value " + val + " for player " + fizzPlayers[i].id);
return false;
}
}
return true;
}

function capitaliseFirstLetter(string) {
return string.charAt(0).toUpperCase() + string.slice(1);
}

function fizzing() {
fizzLoad();
restrictRange();
if (!validateValues()) {
fizzDiv.innerHTML = "Illegal inputs";
return;
}

var table = "";
var rFrom = parseInt(fizzFrom.value);
var rTo = parseInt(fizzTo.value);
var active = [];
var actfact = [];
var actname = [];
var player;

for (var i = 0; i \n";
table += " ValueMessage\n";
for (var i = rFrom; i " + i + "" + msg + "\n";
}
table += "\n";

fizzDiv.innerHTML = table;
}
`h1 {
clear: left;
}

hr {
clear: left;
}

label {
display: inline-block;
float: left;
clear: left;
wid

Loading

This is unconventional:

var fizzLoaded;

function fizzLoad() {
  if (fizzLoaded) {
    return;
  }
  fizzLoaded = true;
  …
}

There isn't any harm in removing the "lock". However, a more elegant way would be to write

window.onload = function fizzLoad() {
    …
};

You can use the onload handler to populate the output immediately using the default settings.

active, actfact, actname

I don't recommend maintaining three parallel arrays like this:

var val = parseInt(player.value);
if (val != 0) {
  active.push(player);
  actfact.push(parseInt(player.value));
  actname.push(capitaliseFirstLetter(player.id));
}

It would be easier to understand with one array of objects:

var val = parseInt(player.value);
if (val != 0) {
  active.push({ name: capitaliseFirstLetter(player.id),
                factor: val });
}

Handling five players

Rather than fetching the five elements by ID:

fizzPlayers = [
    document.getElementById('frodo'),
    document.getElementById('sam'),
    document.getElementById('merry'),
    document.getElementById('pippin'),
    document.getElementById('bilbo')
  ];

It would be better to treat them collectively, so that you don't have to hard-code the five IDs:

fizzPlayers = document.getElementById('players').getElementsByTagName('input');

Furthermore, I don't recommend munging the IDs to obtain the name. One problem is that a future maintainer grepping the code for names will be surprised to find that the names are lowercase in the code, but uppercase in the output. Another problem is that identifiers should be identifiers — they are subject to rules.

Event handling and input validation

I don't recommend using oninput, since it fires too easily. A user could be trying to fix an error, and be interrupted again by an alert in the process. That would be especially frustrating since alerts are modal dialogs. An onchange handler on each input element would be more appropriate.

Layout

Floats are a bit of a pain: once you have any float in the page, you have to ensure that everything surrounding it needs to avoid being reflowed inappropriately. Use them sparingly.

You certainly don't need to float the ` elements.

label {
    display: inline-block;
    width: 150px;
    text-align: left;
}
input {
    display: inline-block;
    float: right;
    text-align: right;
    padding-left: 10px;
    width: 50px;
}
hr, input {
    clear: right;
}

Consider abandoning floats in favour of fixed-width labels, as I've done below. Use of a would also be reasonable.



var fizzFrom, fizzTo, fizzPlayers, fizzDiv;

window.onload = function fizzLoad(event) {
// Associate inputs with their labels (https://stackoverflow.com/a/285608)
var labels = document.getElementsByTagName('label');
for (var i = 0; i \n";
table += " ValueMessage\n";
for (var i = rFrom; i " + i + "" + msg + "\n";
}
table += "\n";

fizzDiv.innerHTML = table;
}
label {
display: inline-block;
width: 100px;
}
input {
padding-left: 10px;
width: 50px;
}

Range From


Range To



Frodo


Sam


Merry


Pippin


Bilbo




Output
`

StackExchange Code Review Q#74543, answer score: 31