I have made a simple calculator that takes the information entered by the user in , then performs an action with two const and gives the result in a selected .
In total, i have 5 inputs and 5 outputs. To each input and output i have 2 const.
Code looks like that:
const suprqMean = 3.93;
const usabilityMean = 4.06;
const suprqSD = 0.29;
const usabilitySD = 0.29;
{
const input = document.querySelector("#formGroupExampleInput");
const log = document.getElementById("#suprq");
input.addEventListener("change", updateValue);
function updateValue(e) {
suprq.textContent = (e.target.value - suprqMean) / suprqSD;
}
}
{
const input = document.querySelector("#formGroupExampleInput1");
const log = document.getElementById("#usability");
input.addEventListener("change", updateValue);
function updateValue(e) {
usability.textContent = (e.target.value - usabilityMean) / usabilitySD;
}
}<form class="leftForm container">
<div class="form-group">
<label for="formGroupExampleInput">SUPR-Q raw</label>
<input type="number" class="form-control" id="formGroupExampleInput" placeholder="SUPR-Q raw">
</div>
<div class="form-group">
<label for="formGroupExampleInput2">Usability raw</label>
<input type="number" class="form-control" id="formGroupExampleInput1" placeholder="Usability raw">
</div>
</form>
<table class="table table-striped table-dark">
<thead>
<tr>
<th scope="col">Z-Score</th>
<th scope="col">Result</th>
</tr>
</thead>
<tbody>
<tr>
<td>SUPR-Q:</td>
<td id="suprq"></td>
</tr>
<tr>
<td>Usability:</td>
<td id="usability"></td>
</tr>
</tbody>
</table>And it works , but the JS code looks awful. Is there any way to optimise the JS code? I want the result to be entered in real time as it works now.
CodePudding user response：
You can combine the two functions into one by checking e.target.id inside, and select both input values at once to loop through and add the event listener:
const suprqMean = 3.93;
const usabilityMean = 4.06;
const suprqSD = 0.29;
const usabilitySD = 0.29;
document.querySelectorAll("[id^=formGroupExampleInput").forEach(el => {
el.addEventListener("change", updateValue);
});
function updateValue(e) {
inpVer = e.target.id === "formGroupExampleInput" ? true : false;
outEl = inpVer ? suprq : usability;
outVal = inpVer ? (e.target.value - suprqMean) / suprqSD : (e.target.value - usabilityMean) / usabilitySD;
outEl.textContent = outVal;
}<form class="leftForm container">
<div class="form-group">
<label for="formGroupExampleInput">SUPR-Q raw</label>
<input type="number" class="form-control" id="formGroupExampleInput" placeholder="SUPR-Q raw">
</div>
<div class="form-group">
<label for="formGroupExampleInput2">Usability raw</label>
<input type="number" class="form-control" id="formGroupExampleInput1" placeholder="Usability raw">
</div>
</form>
<table class="table table-striped table-dark">
<thead>
<tr>
<th scope="col">Z-Score</th>
<th scope="col">Result</th>
</tr>
</thead>
<tbody>
<tr>
<td>SUPR-Q:</td>
<td id="suprq"></td>
</tr>
<tr>
<td>Usability:</td>
<td id="usability"></td>
</tr>
</tbody>
</table>CodePudding user response：
Looks like you are somewhat repeating yourself in that JS code.
The usual remedy for this would be to refactor the repeated code into a reusable function:
const suprqMean = 3.93;
const usabilityMean = 4.06;
const suprqSD = 0.29;
const usabilitySD = 0.29;
function listenForChanges (inputId, logId, output, mean, SD) {
const input = document.querySelector(inputId);
const log = document.querySelector(logId);
input.addEventListener("change", updateValue);
function updateValue(e) {
output.textContent = (e.target.value - mean) / SD;
}
}
listenForChanges("#formGroupExampleInput", "#suprq", suprq, suprqMean, suprqSD)
listenForChanges("#formGroupExampleInput1", "#usability", usability, usabilityMean, usabilitySD)
Something else feels weird though. In your original snippet (and in mine too) there are references to two variables that are never declared: suprq and usability.
But apart from that, do you like this use of a function as a first step to make the code a little bit less "awful"?
If you have many of these, the next step could be to make a data structure for all the input pairs and call this function on each pair with a loop:
const all = [{
inputId: "#formGroupExampleInput",
logId: "#suprq",
output: suprq,
mean: 3.93,
SD: 0.29
}, {
inputId: "#formGroupExampleInput1",
logId: "#usability",
output: usability,
mean: 4.06,
SD: 0.29
}]
all.forEach(listenForChanges)
function listenForChanges ({inputId, logId, output, mean, SD}) {
const input = document.querySelector(inputId);
const log = document.querySelector(logId);
input.addEventListener("change", updateValue);
function updateValue(e) {
output.textContent = (e.target.value - mean) / SD;
}
}
Notice that I changed the function a bit here, having it accept a single object parameter instead of a series of parameters. This is just to make it easier to glue everything together with forEach.
CodePudding user response：
Here's is a 'condensed' version
const v = {
suprqMean: 3.93,
usabilityMean: 4.06,
suprqSD: .29,
usabilitySD: .29
};
["suprq", "usability"].map((t => {
document.querySelector(`#${t}Input`).addEventListener("change", (e => {
document.getElementById(`${t}`).textContent = (e.target.value - v[`${t}Mean`]) / v[`${t}SD`]
}))
}));<form class="leftForm container">
<div class="form-group">
<label for="formGroupExampleInput">SUPR-Q raw</label>
<input type="number" class="form-control" id="suprqInput" placeholder="SUPR-Q raw">
</div>
<div class="form-group">
<label for="formGroupExampleInput2">Usability raw</label>
<input type="number" class="form-control" id="usabilityInput" placeholder="Usability raw">
</div>
</form>
<table class="table table-striped table-dark">
<thead>
<tr>
<th scope="col">Z-Score</th>
<th scope="col">Result</th>
</tr>
</thead>
<tbody>
<tr>
<td>SUPR-Q:</td>
<td id="suprq"></td>
</tr>
<tr>
<td>Usability:</td>
<td id="usability"></td>
</tr>
</tbody>
</table>CodePudding user response：
Others have shown ways to use a configuration array to generate your handlers.
You could also generate the HTML from the same configuration, which means that adding a new type is just the matter of one more configuration entry:
const config = [
{title: 'SUPR-Q', mean: 3.93, sd: 0.29, input: 'formGroupExampleInput', log: 'suprq'},
{title: 'Usability', mean: 4.06, sd: 0.29, input: 'formGroupExampleInput1', log: 'usability'}
]
document.getElementById('main').innerHTML = `
<form >
${config .map (({title, input}) => ` <div >
<label for="${input}">${title} raw</label>
<input type="number" id="${input}" placeholder="${title} raw">
</div>`) .join ('\n' )}
</form>
<table >
<thead><tr><th scope="col">Z-Score</th><th scope="col">Result</th></tr></thead>
<tbody>${config .map (({title, output}) =>
`<tr><td>${title}</td><td id="${output}"></td></tr>`).join('\n ')
}</tbody>
</table>`
config .forEach (({title, mean, sd, input, log}) => {
const inputElt = document.getElementById(input);
const logElt = document.getElementById(log);
inputElt.addEventListener("change", function (e) {
logElt.textContent = (e.target.value - mean) / sd;
})
})<div id="main"></div>I also switched to a consistent getElementById. We could just as easily use querySelector here, but mixing them feels odd when we're always getting ids.