Welcome to the Treehouse Community

Want to collaborate on code errors? Have bugs you need feedback on? Looking for an extra set of eyes on your latest project? Get support with fellow developers, designers, and programmers of all backgrounds and skill levels here with the Treehouse Community! While you're at it, check out some resources Treehouse students have shared here.

Looking to learn something new?

Treehouse offers a seven day free trial for new students. Get access to thousands of hours of content and join thousands of Treehouse students and alumni in the community today.

Start your free trial

JavaScript

Javascript isNaN assistance

Hello! I could use some help assessing my code. For the life of me I can't get this if statement to work properly. I've tried everything I can think of to no avail. Any ideas?

var processEntry = function () {
  var x = parseFloat($("income").value);
  if (isNaN(x)) {
    window.alert("no!");
  }
  else {
    calculateTax();
  }
};

I should mention that the property .value would work if you were coding this in plain javascript and had retrieved the input element. But if you're using jQuery then you would need .val()

Hi whalien52,

I apologize for adding to the confusion when I mistakenly thought you were using jQuery. Where have you seen the $ used for this purpose?

Please disregard everything that was previously said.

6 Answers

Hi!

Technically, the problem with your code is that you're not calling processEntry() ever. But, this is just a symptom of a larger issue: a lack of readability. One of the paramount concepts you want to keep in mind, as you progress in your coding endeavors, is the idea that each function you write should ideally have one purpose — one reason for existence and one reason for changing. This one principle will drive you a long way towards maintaining your sanity, while coding.

Below is a commented, working version of your code. I think the best way to go about showing you where you went wrong is to let you take a look at the code and post a reply with any questions you may have. So look through it, mess around with things, break the structure and figure out how to fix it again. Maybe you'll find a better way of doing things. In any case, there's no one right path when it comes to software architecture. Just keep in mind that readability trumps all ;)

function isNotAnIncome(income) {
    // we're expecting income to be a string here
    // parseFloat will return NAN if income is ... well... not a number
    // isNaN is just a shorter way of checking if a thing is not a number
    return isNaN(parseFloat(income))
}

// This function is going to take as input a string that represents an income
// And it'll return a Number that represent the corresponding tax percentage
function calculateTaxPercentage(income) {
    // Note we don't need to define a new variable...
    // ... because function parameters, in JavaScript, are mutable things
    // You can if you want though; it'd just be redundant
    income = parseFloat(income)

    // Actual calculations
    // Note how you don't need the else blocks, since we're exiting early each time
    if (income <= 9275) {
        return income * .10
    }

    if (income >= 9275 && income <= 37650) {
        return income * .15
    }

    if (income >= 37651 && income <= 91150) {
        return income * .25
    }

    if (income >= 91151 && income <= 190150) {
        return income * .28
    }

    if (income >= 191151 && income <= 413350) {
        return income * .33
    }

    if (income >= 413351 && income <= 415050) {
        return income * .35
    }

    return income * .396
}

// Get your button
// You don't really need that "$" shortcut...
// Remember: Readability > Everything
const $button = document.getElementById("calculate")

// Attach an event listener
// I like these better than "onWhatever" handlers
// Among other things, they're way more readable
$button.addEventListener("click", function (event) {
    // Get your other DOM elements
    const $income = document.getElementById("income")
    const $tax = document.getElementById("tax")

    // Get the value from the $income field
    const income = $income.value

    // Validate the income
    if (isNotAnIncome(income)) {
        // You can do your alert() here instead, if you want
        throw "Uh-uh :("
    }

    // You can set the value of your $tax field now...
    // ... using the return value from calculateTaxPercentage()
    // ... and also you don't have to keep repeating your .toFixed()
    $tax.value = calculateTaxPercentage(income).toFixed(2)
})

The if conditions can be shortened down too.

For example, on the second if condition you don't have to check if the income is greater than 9275. If you've reached that far then it has to be greater than 9275.

if (income <= 37650) {
Andrew McCormick
Andrew McCormick
17,730 Points

To get the value of a jQuery object you use .val() .value will return undefined

Thanks, I completely missed that.

I tried changing that as well to no avail. I'll include my code in its entirety below. However, it's worth mentioning once again that my only issue seems to be with doing something with isNaN. Am I using it incorrectly?

"use strict";
var $ = function (id) {
    return document.getElementById(id);
};


var processEntry = function () {
  var x = parseFloat($("income").value);
  if (isNaN(x)) {
    window.alert("no!");
  }
  else {
    calculateTax();
  }
};

var calculateTax = function () {
var income = parseFloat($("income").value);
  if (income <= 9275) {
    var percent = income * .10;
    $('tax').value = percent.toFixed(2);
  }
  else if (income >= 9275 && income <= 37650 ) {
    var percent = income * .15;
    $('tax').value = percent.toFixed(2);
  }
  else if (income >= 37651 && income <= 91150) {
    var percent = income * .25;
    $('tax').value = percent.toFixed(2);
  }
  else if (income >= 91151 && income <= 190150) {
    var percent = income * .28;
    $('tax').value = percent.toFixed(2);
  }
  else if (income >= 191151 && income <= 413350) {
    var percent = income * .33;
    $('tax').value = percent.toFixed(2);
  }
  else if (income >= 413351 && income <= 415050) {
    var percent = income * .35;
    $('tax').value = percent.toFixed(2);
  }
  else {
    var percent = income * .396;
    $('tax').value = percent.toFixed(2);
  }
};


window.onload = function () {
    $("calculate").onclick = calculateTax;
};

Your use of isNaN appears to be correct. If the string can't be parsed as a float then NaN will be returned.

isNaN(NaN) will be true and you'll get your alert.

In the code that you have posted here, you don't have either of the changes that have been suggested. You're not using val() instead of value and you don't have any id selectors.

Have you put in any console.log() statements anywhere to see if you're getting the values that you think you're getting?

I attempted sending x to a console log. I can't get anything to print at all in the console. Even if I send a string through, my console stays empty.

var processEntry = function () {
    console.log("string");
    var x = parseFloat($('#income').val); 
    console.log(x);
    calculateTax();
};

It calculates my tax bracket but nothing else seems to be functioning inside of this block.

I ended up amending my calculateTax function. Is this bad practice?

"use strict";
var $ = function (id) {
    return document.getElementById(id);
};


var processEntry = function () {
    calculateTax();
};

var calculateTax = function () {
var income = parseFloat($("income").value);
  if (income <= 9275) {
    var percent = income * .10;
    $('tax').value = percent.toFixed(2);
  }
  else if (income >= 9275 && income <= 37650 ) {
    var percent = income * .15;
    $('tax').value = percent.toFixed(2);
  }
  else if (income >= 37651 && income <= 91150) {
    var percent = income * .25;
    $('tax').value = percent.toFixed(2);
  }
  else if (income >= 91151 && income <= 190150) {
    var percent = income * .28;
    $('tax').value = percent.toFixed(2);
  }
  else if (income >= 191151 && income <= 413350) {
    var percent = income * .33;
    $('tax').value = percent.toFixed(2);
  }
  else if (income >= 413351 && income <= 415050) {
    var percent = income * .35;
    $('tax').value = percent.toFixed(2);
  }
  else if(isNaN(income)) {
    window.alert("Please enter a number.");
    return false;
  }
  else {
    var percent = income * .396;
    $('tax').value = percent.toFixed(2);
  }
};


window.onload = function () {
    $("calculate").onclick = calculateTax;
};
Steven Parker
Steven Parker
229,582 Points

I think folks have gotten confused here by your use of $ as a shorthand for getElementById. It looks deceptively similar to jQuery.

So all the comments about jQuery and .val() don't really apply here.

Is this most recent version doing what you want but you want to see if it can be written better?

Thank you everyone for all of the assistance! It's greatly appreciated! ^-^

Ah, that all makes sense. Mikis Woodwinter Thank you so much for your input! I feel kind of ridiculous that I never thought about calling the function. That would explain an awful lot. I appreciate the line by line breakdown as well. Super helpful!

Jason Anello I had been wondering if there was a shorter way to code those statements. It started feeling really redundant. I'll keep this in mind for my next project!