This exercise with a test suite is from here. I did not intend to use ES6 features, it's plain old JavaScript. That's why I seek advice more on the good practices, performance side, than modern syntax. Basically anything to make this code more elegant, without ES6.
// Constructor
var Binary = function(binary) {
this.binary = binary;
this.base = 2;
};
// Transform the string input in a reversed array
Binary.prototype.reverseInput = function() {
return this.binary.split('').reverse();
};
// Handle the conversion to decimal
Binary.prototype.toDecimal = function() {
var output = 0;
binaryArray = this.reverseInput();
if (this.invalidInput()) {
return output;
}
for (var i = 0, j = binaryArray.length; i < j; i++) {
output += binaryArray[i] * Math.pow(this.base, i);
}
return output;
}
// Check if any character is not a 1 or a 0
Binary.prototype.invalidInput = function() {
for (var i = 0, j = this.binary.length; i < j; i++) {
if (this.binary[i] != '0' && this.binary[i] != '1') {
return true;
}
}
}
module.exports = Binary;
1 Answer 1
Flawed exercise
Before I begin, I would like to point out that the exercise is flawed:
The task is not really converting the input from a binary to a decimal representation. Rather, it is to convert it to a number, whose conventional string representation is rendered in base 10. The test suite included this test:
xit('11010 is decimal 26', function() { expect(new Binary('11010').toDecimal()).toEqual(26); });
... which could also have been written as:
xit('11010 is twenty-six', function() { expect(new Binary('11010').toDecimal()).toEqual(0x1a); });
It is bad practice to return
0
fornew Binary('junk').toDecimal()
. Acceptable results would be to either throw an exception or returnNumber.NaN
.
Review
Your invalidInput
function either returns true
or falls off the end. Good practice would be to name it isInvalidInput
, and have it explicitly return true
or false
. Better yet, reverse the sense and call it isValidInput
. Even better, don't expose the validation function as a method at all — you don't really need one.
this.binary.split('').reverse()
and Math.pow(...)
seem extravagant.
Suggested solution
// Constructor
var Binary = function(binary) {
this.binary = binary;
this.base = 2;
};
Binary.prototype.toDecimal = function() { // <-- Method name is a misnomer!
var output = 0;
for (var i = 0; i < this.binary.length; i++) {
output *= 2;
switch (this.binary.charAt(i)) {
case '0': break;
case '1': output += 1; break;
default: return 0; // <-- This sucks, but the test suite requires it!
}
}
return output;
}
Explore related questions
See similar questions with these tags.