Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

##Rewrite

Rewrite

##Rewrite

Rewrite

edited body
Source Link
Blindman67
  • 22.8k
  • 2
  • 16
  • 40
"use strict";
const binaryInput = document.getElementById("binaryInput");
const decimalOutput = document.getElementById("decimalOutput");
const inputWarning = document.getElementById("inputWarning");
binaryInput.addEventListener('change', update);
binaryInput.addEventListener('keyup', update);
binaryInput.focus();
const WARNING_TIME = 2000; // in ms
var warningTimer;
function hidWarning() {
 inputWarning.classList.add("hideWarning");
}
function showWarning() {
 clearTimeout(warningTimer);
 warningTimer = setTimeout(hidWarning, WARNING_TIME);
 inputWarning.classList.remove("hideWarning");
}
function update() {
 var value = binaryInput.value;
 if (/[^01]/g.test(value)){
 binaryInput.value = value = value.replace(/[^01]/g, "");
 showWarning();
 }
 decimalOutput.textContent = value === "" ? "" : parseInt(value, 2);
}
body {
 background: #2e2e2e;
 color: #fff;
 font-family: Heebo, sans-serif;
}
.form-body {
 display: flex;
 flex-direction: column;
 text-align: center;
}
input {
 margin: 0 auto;
 width: 100%;
 max-width: 300px;
 border: 0.125rem solid rgba(255,255,255,0.25);
 background: rgba(255,255,255,0.05);
 color: #fff;
 font-size: 2rem;
 text-align: center;
 font-weight: 300; 
}
.inputWarning { color: #F88 }
.hideWarning { display: none }
<div class="form-body">
 <h2>Binary to Decimal Calculator</h1>h2>
 <label for="binaryInput">Enter a binary value<span id="inputWarning" class="inputWarning hideWarning"> [Only valid digits 0 and 1 permitted]</span></label>
 <input id="binaryInput" type="text" size="8" maxlength="8">
 <h3>Decimal Value: <span id="decimalOutput"></span></h2>h3>
</div>
"use strict";
const binaryInput = document.getElementById("binaryInput");
const decimalOutput = document.getElementById("decimalOutput");
const inputWarning = document.getElementById("inputWarning");
binaryInput.addEventListener('change', update);
binaryInput.addEventListener('keyup', update);
binaryInput.focus();
const WARNING_TIME = 2000; // in ms
var warningTimer;
function hidWarning() {
 inputWarning.classList.add("hideWarning");
}
function showWarning() {
 clearTimeout(warningTimer);
 warningTimer = setTimeout(hidWarning, WARNING_TIME);
 inputWarning.classList.remove("hideWarning");
}
function update() {
 var value = binaryInput.value;
 if (/[^01]/g.test(value)){
 binaryInput.value = value = value.replace(/[^01]/g, "");
 showWarning();
 }
 decimalOutput.textContent = value === "" ? "" : parseInt(value, 2);
}
body {
 background: #2e2e2e;
 color: #fff;
 font-family: Heebo, sans-serif;
}
.form-body {
 display: flex;
 flex-direction: column;
 text-align: center;
}
input {
 margin: 0 auto;
 width: 100%;
 max-width: 300px;
 border: 0.125rem solid rgba(255,255,255,0.25);
 background: rgba(255,255,255,0.05);
 color: #fff;
 font-size: 2rem;
 text-align: center;
 font-weight: 300; 
}
.inputWarning { color: #F88 }
.hideWarning { display: none }
<div class="form-body">
 <h2>Binary to Decimal Calculator</h1>
 <label for="binaryInput">Enter a binary value<span id="inputWarning" class="inputWarning hideWarning"> [Only valid digits 0 and 1 permitted]</span></label>
 <input id="binaryInput" type="text" size="8" maxlength="8">
 <h3>Decimal Value: <span id="decimalOutput"></span></h2>
</div>
"use strict";
const binaryInput = document.getElementById("binaryInput");
const decimalOutput = document.getElementById("decimalOutput");
const inputWarning = document.getElementById("inputWarning");
binaryInput.addEventListener('change', update);
binaryInput.addEventListener('keyup', update);
binaryInput.focus();
const WARNING_TIME = 2000; // in ms
var warningTimer;
function hidWarning() {
 inputWarning.classList.add("hideWarning");
}
function showWarning() {
 clearTimeout(warningTimer);
 warningTimer = setTimeout(hidWarning, WARNING_TIME);
 inputWarning.classList.remove("hideWarning");
}
function update() {
 var value = binaryInput.value;
 if (/[^01]/g.test(value)){
 binaryInput.value = value = value.replace(/[^01]/g, "");
 showWarning();
 }
 decimalOutput.textContent = value === "" ? "" : parseInt(value, 2);
}
body {
 background: #2e2e2e;
 color: #fff;
 font-family: Heebo, sans-serif;
}
.form-body {
 display: flex;
 flex-direction: column;
 text-align: center;
}
input {
 margin: 0 auto;
 width: 100%;
 max-width: 300px;
 border: 0.125rem solid rgba(255,255,255,0.25);
 background: rgba(255,255,255,0.05);
 color: #fff;
 font-size: 2rem;
 text-align: center;
 font-weight: 300; 
}
.inputWarning { color: #F88 }
.hideWarning { display: none }
<div class="form-body">
 <h2>Binary to Decimal Calculator</h2>
 <label for="binaryInput">Enter a binary value<span id="inputWarning" class="inputWarning hideWarning"> [Only valid digits 0 and 1 permitted]</span></label>
 <input id="binaryInput" type="text" size="8" maxlength="8">
 <h3>Decimal Value: <span id="decimalOutput"></span></h3>
</div>
Source Link
Blindman67
  • 22.8k
  • 2
  • 16
  • 40

  • You should use the directive "use strict" that will place the JavaScript context into strict mode. This will throw errors for some common bad practices.

  • Always declare variables as const, var, or let. If you are in strict mode you will get an error if you don't.

  • Don't use alerts or prompts as there is no way to know if they are actually displayed (clients can turn them off) and they are very annoying.

  • The key event properties KeyboardEvent.keyCode and KeyboardEvent.charCode have been depreciated and you should not use them. Use KeyboardEvent.code or KeyboardEvent.key instead

  • Rather than filter the input via the keybpoard events, listen to the input's keyup and change events, removing bad characters automatically. Use a CSS rule to unhide a warning and a JavaScript setTimeout to hide it again

    Filtering keyboard events means you need to check many keys that are valid (left, right, backspace, delete, etc...) which is just unneeded complexity.

  • Don't wait for the user to click "Convert to Decimal", display the output automatically. This makes it a lot friendlier to use.

  • JavaScript can convert binary strings to decimal for you using parseInt. The second argument is the radix (AKA base) of the number that is being parsed.

  • If you are just setting text (no HTML) use the elements textContent rather than innerHTML

##Rewrite

The rewrite is following the points you have set-out in your question. I have not implemented how your code differs from these points.

The rewrite uses

  • a RegExp to test and filter the input.
  • a ternary expression to create the decimal value as parseInt will return NaN for empty strings. The ternary checks if the string is empty evaluating to "" or if there is a number the ternary evaluates to the decimal value.
  • HTMLInputElement.focus (inherited from HTMLElement) to focus the input element when loaded.

I have modified the HTML and CSS to fit the snippet window a little better.

"use strict";
const binaryInput = document.getElementById("binaryInput");
const decimalOutput = document.getElementById("decimalOutput");
const inputWarning = document.getElementById("inputWarning");
binaryInput.addEventListener('change', update);
binaryInput.addEventListener('keyup', update);
binaryInput.focus();
const WARNING_TIME = 2000; // in ms
var warningTimer;
function hidWarning() {
 inputWarning.classList.add("hideWarning");
}
function showWarning() {
 clearTimeout(warningTimer);
 warningTimer = setTimeout(hidWarning, WARNING_TIME);
 inputWarning.classList.remove("hideWarning");
}
function update() {
 var value = binaryInput.value;
 if (/[^01]/g.test(value)){
 binaryInput.value = value = value.replace(/[^01]/g, "");
 showWarning();
 }
 decimalOutput.textContent = value === "" ? "" : parseInt(value, 2);
}
body {
 background: #2e2e2e;
 color: #fff;
 font-family: Heebo, sans-serif;
}
.form-body {
 display: flex;
 flex-direction: column;
 text-align: center;
}
input {
 margin: 0 auto;
 width: 100%;
 max-width: 300px;
 border: 0.125rem solid rgba(255,255,255,0.25);
 background: rgba(255,255,255,0.05);
 color: #fff;
 font-size: 2rem;
 text-align: center;
 font-weight: 300; 
}
.inputWarning { color: #F88 }
.hideWarning { display: none }
<div class="form-body">
 <h2>Binary to Decimal Calculator</h1>
 <label for="binaryInput">Enter a binary value<span id="inputWarning" class="inputWarning hideWarning"> [Only valid digits 0 and 1 permitted]</span></label>
 <input id="binaryInput" type="text" size="8" maxlength="8">
 <h3>Decimal Value: <span id="decimalOutput"></span></h2>
</div>

default

AltStyle によって変換されたページ (->オリジナル) /