Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 2 down vote favorite

I am aware of it being frowned upon to do something like write C# in JavaScript. (See Write Cobol in any language if you don't know what I'm talking about.) But as a judgement call I think we could stand to have a relatively simple check for values that are null or empty. So I'm Looking for feedback on this implementation of String.isNullOrEmpty.

String.isNullOrEmpty = function (value) {
    try {
        value = value.toString();
    } catch (e) {
        return true;
    }
    return (!value || value == undefined || value == "" || value.length == 0);
}

Revision 1:

String.isNullOrEmpty = function (value) {
    return (!value || value == undefined || value == "" || value.length == 0);
}
share|improve this question
1  
Aren't the first checks redundant after you've already tried to call toString on value? – user786653 Oct 25 '11 at 19:50
Possibly.... I was thinking if I were to call a "static" String.isNullOrEmpty(). Not sure if I need it. – Terrance Oct 25 '11 at 20:04
That revision is no good, see this and other posts. You've radically changed the meaning of the function. – user786653 Oct 25 '11 at 20:59
Not following you on that second comment. "You've radically changed the meaning of the function" I just removed that first try, catch. I didn't change any comparison operators. Maybe you should give me your thoughts in a full answer form. – Terrance Oct 25 '11 at 21:13
2  
Would not a return !value; suffice? – James Khoury Oct 26 '11 at 1:16
show 7 more comments

3 Answers

up vote 6 down vote accepted

Starting with:

return (!value || value == undefined || value == "" || value.length == 0);

Looking at the last condition, if value == "", it's length MUST be 0. Therefore drop it:

return (!value || value == undefined || value == "");

But wait! In JS, an empty string is false. Therefore, drop value == "":

return (!value || value == undefined);

And !undefined is true, so that check isn't needed. So we have:

return (!value);

And we don't need parentheses:

return !value

Q.E.D.

share|improve this answer
i must say... bravo – Hexxagonal Oct 31 '11 at 15:43
@ndp Gotta give it to you. That was complete and concise. Nice. – Terrance Nov 2 '11 at 20:37
up vote 5 down vote

There are just a few revisions I would make.

First, always use === instead of == in Javascript. You can read more about that on Stack Overflow.

Second, since undefined is mutable, I would reccomend using

typeof value === "undefined"

instead of 

value === undefined

Third, I would remove the !value and value === "" conditions. They are redundant.

My Revision

I would use a slightly different approach than you:

String.isNullOrEmpty = function(value) {
  return !(typeof value === "string" && value.length > 0);
}

This checks if the type of the value is "string" (and thus non-null and not undefined), and if it is not empty. If so, it is not null or empty.

Note that this returns true for non-string inputs, which might not be what you want if you wanted to throw an error for an unexpected input type.

share|improve this answer
up vote 1 down vote

You should use Object.prototype.isNullOrEmpty = function() { alert(this) }. This ties it to the String object in all instances. This would start to give you access to use strings and variables like "".isNullOrEmpty or var x = null; x.isNullOrEmpty();

If your intent is to use it as a function to pass in variables: String.isNullOrEmpty();

share|improve this answer
Except some people don't like extending the native prototypes. – Peter Olson Oct 28 '11 at 15:17

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.