-
Notifications
You must be signed in to change notification settings - Fork 803
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Tip #10 delusion #62
Comments
@rocketinventor, in practice I saw lot of people with more than 3 years JS experience setting undefined to property meaning it is empty. And also tons of other mistakes that should not be done. So if you want to say it is a right way to assign null to param when you want to say it is empty - that's correct, if you want to say it is a good practice - also correct, but saying:
is completely wrong, especially when inexperienced rookies reading these JS tips and have at least few places in their code where they assign undefined |
@igor-galchevsky: you have a valid point, but the problem you've raised also applies to the other mentioned methods. var o = {
test: undefined
};
typeof o.test === 'undefined'; // true
o.hasOwnProperty('test'); // true
'test' in o; // true If you look at the ECMAScript source code, the hasOwnProperty method is ultimately checking for undefined. I will add to this point though, that it is often agreed that native JavaScript methods are slower than doing the check yourself. There's quite a lot of extra handling that goes on in the @loverajoel: A small point that may be worth mentioning about this as well. If you use |
Why don't we write up a tip for this, then? |
@rocketinventor, what do you mean "when" ? :) |
my two cents => https://gist.github.com/dfkaye/c4da8da17b03f5fad4ff |
I think that the discussion here is if people define empty vars as Thanks for this rich discussion 😄 |
It is definitely not ok to use Regarding setting empty references:
(function( undefined ) {}()) -> (function( a ) {}())
|
@FagnerMartinsBrack I'm not agree with your last point and why do you says that I will make a pr and remove this check and add a link to this discussion. |
If I understand the complaint, it's that
|
fixed d885967 |
@dfkaye that's right. My point is that it is just unsafe to check property existence using |
@loverajoel @igor-galchevsky
|
The previous bullet point make it clear that if you care for bytes you can use
It depends on what you consider "empty references". "If something works sometimes, and sometimes it does not, and there is a better option, always use the better option". If one does not agree with this principle, then there is no point to continue the discussion.
When you say var obj = { a: undefined };
obj.hasOwnProperty( "a" ); // true
typeof obj.a; // undefined
var obj = {};
obj.hasOwnProperty( "a" ); // false
typeof obj.a; // undefined See that both |
If |
Is it not easier to just do |
Why don't you use hasOwnProperty like @FagnerMartinsBrack said! It's simplest and most obvious solution. Using typeof to check existence is silly |
Hey @Kerndog73, @FagnerMartinsBrack, can you guys submit a PR with the required changes then we can review it and merge it. |
In example you have this object
and this check
and then these words saying that that kind of check is ok.
Actually it is not ok, consider this example:
Which clearly shows that
typeof myObject['name'] === 'undefined'
is not checking existence of property, but checks type of that property valueThe text was updated successfully, but these errors were encountered: