0
\$\begingroup\$

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;
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Jul 27, 2016 at 20:36
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

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 for new Binary('junk').toDecimal(). Acceptable results would be to either throw an exception or return Number.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;
}
answered Jul 27, 2016 at 21:36
\$\endgroup\$
0

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.