Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: Doesn't allow defaultValue = undefined in firstOrDefault #47

Closed
bendtherules opened this issue Mar 8, 2018 · 2 comments
Closed
Assignees

Comments

@bendtherules
Copy link

bendtherules commented Mar 8, 2018

Enumerable.prototype.firstOrDefault = function (predicate, defaultValue) {
        if (predicate) {
            if (typeof predicate === Types.Function || typeof Utils.createLambda(predicate) === Types.Function)
                return this.where(predicate).firstOrDefault(null, defaultValue);

                defaultValue = predicate;
        }

        defaultValue = defaultValue || null; // THIS LINE
        ...

Maybe we should remove this line from firstOrDefault to allow undefined as default value?

It is specially important because the user might pass in undefined explicitly for the parameter defaultValue (which is the conventional missing value in JS), and this undefined value will get overloaded with null.

Even the type definition doesnt indicate that the return type might be T or null. In fact, type definition doesn't allow passing in null explicitly as defaultValue (which would make things more explicit and understandable for users).

The user has no way of knowing this unless they actually look into the code.

@mihaifm
Copy link
Owner

mihaifm commented Mar 8, 2018

You seem to be using an earlier version...can you check (in the latest version from git) if you can pass null as the defaultValue? You should be able to do so. Otherwise we may need to fix it.

@bendtherules
Copy link
Author

bendtherules commented Mar 9, 2018

You are correct, the type definition is fixed in master.
But the problem with the logic still exists https://github.com/mihaifm/linq/blob/master/linq.js#L1865 . You still can't set undefined as defaultvalue. I would suggesting changing the default of defaultValue to undefined from null, as that is the common way to represent missing in JS.

@M0ns1gn0r M0ns1gn0r self-assigned this Mar 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants