codetalker
codetalker

Reputation: 586

Why is the error not thrown in this code?

This is my code (Javascript):

function getNumbers(){
    var numberString = document.getElementById("numbers").value;
    var actualNumbers = [];
    var flowIndex = 0;
    for(let i = 0; i < numberString.length; i++){
        if(numberString.charAt(i) != " " && numberString.charAt(i) != "\," && numberString.charAt(i) != "\t"){
            actualNumbers[flowIndex] = parseInt(numberString.charAt(i));
            flowIndex++;
        }
        else continue;
    }
    return actualNumbers;
}

function division(){
    try{
        var answer = getNumbers()[0] / getNumbers()[1];
        if(getNumbers()[1] == 0)
            throw "bad";
    }
    catch(error){
        throw error.description + "Division by zero error";
    }
    finally{
        return answer;
    }
}

I have a function getNumbers() which returns an array, with array[0] = 1 and array[1] = 0. Now, I want to throw an exception "bad" when array[1] == 0. But, neither the try exception nor the catch exception is being thrown, but the finally clause is working. What is the problem?

NOTE: On division by zero, no exception is being thrown, instead answer is coming out to be Infinity. getNumbers() is working properly.

Upvotes: 0

Views: 150

Answers (1)

T.J. Crowder
T.J. Crowder

Reputation: 1074266

The exception is getting thrown, but then you're suppressing the exception by doing this:

finally {
    return answer;
}

The finally clause gets final say. If you return from it, that suppresses the exception and makes the function complete normally.

One way to fix it is to remove the finally clause and put the return answer; inside your try.


Just FWIW, some other notes as comments:

function getNumbers(){
    var numberString = document.getElementById("numbers").value;
    var actualNumbers = [];
    var flowIndex = 0;

    // You might consider splitting the string into an array of one-character strings so you
    // aren't constantly calling a method (`charAt`), like this:
    //     `var chars = numberString.split("");`
    // Then index into `chars`

    // N.B. `let` is an ES2015 (ES6) feature not all JavaScript engines have as it's new;
    // the rest of your code is using the older `var`
    // --v
    for (let i = 0; i < numberString.length; i++){
        // No need to escape the comma --------------------------------v
        if(numberString.charAt(i) != " " && numberString.charAt(i) != "\," && numberString.charAt(i) != "\t"){
            actualNumbers[flowIndex] = parseInt(numberString.charAt(i));
            flowIndex++;
        }
        // No need for the `else continue;` at all
        else continue;
        // In the above, you regularly call `charAt` four times when once would have been sufficient.
        // You might also consider a `switch`
    }
    return actualNumbers;
}

function division(){
    try{
        // Rather than calling `getNumbers` three separate times, call it once and remember its return value
        // doing the calculation should be AFTER checking [1] for 0, not before
        var answer = getNumbers()[0] / getNumbers()[1];
        if(getNumbers()[1] == 0)
            throw "bad";                    // Recommend using Error, e.g.: `throw new Error("bad")`
        // Move the `return answer;` here
    }
    catch(error){
        // You've thrown a string, it doesn't have a `description` property
        // Separately: Why throw something above, just to catch it here and throw something else?
        throw error.description + "Division by zero error";
    }
    // Remove the finally
    finally{
        return answer;
    }
}

Again just FWIW, I'd probably put responsibility for getting the value from the input in division (or even in the thing calling division) rather than in getNumbers, and use /\d/.test(...) to test if a character is a digit, since there are lots of non-digits that aren't " ", ",", or "\t". And once we know they're digits, we can use +ch instead of parseInt to convert them (but with this input, it's just a style choice [well, there's a performance implication, but 99.99% of the time, that doesn't matter]).

So perhaps:

function getNumbers(str) {
    var numbers = [];
    str.split("").forEach(function(ch) {
        if (/\d/.test(ch)) {
            numbers.push(+ch);
        }
    });
    return numbers;
}

function division() {
    var numbers = getNumbers(document.getElementById("numbers").value);
    if (numbers[1] == 0) {
        throw new Error("Division by zero error");
    }
    return numbers[0] / numbers[1];
}

Or with an alternate getNumbers that is more concise, but makes more loops through the input (which usually doesn't matter):

function getNumbers(str) {
    return str.split("")
        .filter(function(ch) { return /\d/.test(ch); })
        .map(function(ch) { return +ch; });
}

function division() {
    var numbers = getNumbers(document.getElementById("numbers").value);
    if (numbers[1] == 0) {
        throw new Error("Division by zero error");
    }
    return numbers[0] / numbers[1];
}

Upvotes: 4

Related Questions