2

I have some code which looks like following: 

$scope.query.toLowerCase() != 1

If query is not defined - there is an error in the console. Actually the code still produces the result that I want, but what is the right approach of dealing with error? Of course I can check the the variable in advance - but that will lead to more code, not sure of the benefits of hiding errors by having to read more code.

Niklesh Raut
  • 34,013
  • 16
  • 75
  • 109
naneri
  • 3,771
  • 2
  • 29
  • 53
  • 1
    Possible duplicate of [Detecting an undefined object property](http://stackoverflow.com/questions/27509/detecting-an-undefined-object-property) – Hacketo Feb 11 '16 at 10:08
  • 2
    @Hacketo Does not seem like a duplicate to me. – Derek 朕會功夫 Feb 11 '16 at 10:09
  • You should check it, and throw your own error and then deal with the error in your program, especially if query being defined is a direct consequence of a user providing the proper input. If this should never happen with any type of user input, then let it fail as is. – Neil Feb 11 '16 at 10:11
  • Come on, I do know how to check if variable is undefined. The question is - what is the shortes way to write the code. Checking will make the code longer. – naneri Feb 11 '16 at 10:11
  • Well, you should ALWAYS add defensive code and test variable definition. – fredmaggiowski Feb 11 '16 at 10:26

3 Answers3

3

Of course I can check the the variable in advance - but that will lead to more code

Writing more code doesn't make your code bad, unless you are in a code golf competition. If you don't handle your errors and edge cases, then your program will not be reliable. If a line of code throws an error, your code might even terminate prematurely.

An alternative way to deal with possible undefined properties is to define a default value instead of checking if it is undefined.

($scope.query || "").toLowerCase() != 1       // this is enough to fix your expression

or more formally

($scope.query === undefined ? "" : $scope.query).toLowerCase() != 1

This obviously depends on what your purpose is.

Another example:

function add(a, b){
    return a + (b || 0);  // if b is falsy, assume b is 0
}

add(1, 2);   // 3
add(1);      // 1

Shortcircuiting is very useful in some cases, but make sure you know exactly how it works because misusing it will create unexpected behaviors.

Derek 朕會功夫
  • 92,235
  • 44
  • 185
  • 247
  • Thanks for detailed answer. Of course I could solve the error displaying, but did not want to go with a quick solution and your answer helps a lot to improve my code readabilty. – naneri Feb 11 '16 at 10:26
  • "define a default value instead of checking if it is undefined". your example check `$scope.query` value to use a default value .. – Hacketo Feb 11 '16 at 10:31
  • @Hacketo To clarify what I meant, is instead of using an `if` and introduce a whole new block of code, short circuiting is a faster shortcut to get around it. – Derek 朕會功夫 Feb 11 '16 at 10:33
  • @Derek朕會功夫 You can use inline `if`: `$scope.query && $scope.query.toLowerCase()` this doesn't add any block of code, thought it may harm readability – fredmaggiowski Feb 11 '16 at 10:50
1

Its bad practice to do ...

try {
    $scope.query.toLowerCase() != 1
} catch(e) {
    // do nothing
}

since this obscures errors and if an unexpected error happens you wont know about it which makes debugging a nightmare.

The approved practice is to write a few extra lines of defensive code to protect against the scenario that would lead to an error. Yes its more verbose but its better than having a codebase which is a nightmare to debug.

danday74
  • 52,471
  • 49
  • 232
  • 283
0

in general: I'm outsorcing this task into a tiny helper:

function string(v){ return v==null? "": String(v) }

and I can be sure, that the output is always a String (Type-safety).

string($scope.query).toLowerCase() != 1

In this special case? you are checking against a number, you don't need the toLowerCase()

+$scope.query !== 1

would be even better in this case; an no need to deal with null or undefined.

Thomas
  • 3,513
  • 1
  • 13
  • 10