Should findSingle has the default predicate?

[kt3k]

findSingle in collections module has the default selector predicate of (_) => true. Does this make sense? What's the use case?

context of this discussion: #1166 (comment)

[getspooky]

@kt3k yes there is no need to add this default selector of (_) => true , it does not make sense.
I will push a fix for this.

[LionC]

@kt3k yes there is no need to add this default selector of (_) => true , it does not make sense.

I will push a fix for this.

That is a very opinionated statement without stating reasons^^

Keep in mind that the default selector has been dorectly copied from the inspired module - which seems to see a use case for it (one I have used amd encountered myself several times).

I do not see how rempving it adds any value right now - at least not without actually talking about why.

[getspooky]

@LionC thanks for feedback, let's take this example and discuss:

const bookings = [
  { month: 'January', active: false },
  { month: 'March', active: false },
  { month: 'June', active: true },
];

const dd = findSingle(bookings);

It will return undefined but there is no given condition passed. This think could be useful if we specify field like

const dd = findSingle(bookings, 'active');

even if we pass one value like

const bookings = ["January", "March", "June"];
const dd = findSingle(bookings);

it does not make sense

@kt3k any feedback ?

[kt3k]

The default selector (predicate) can be used for getting the single element from a signleton array

const bookings = [{ month: 'January', active: false }];
const item = findSingle(bookings);
console.log(item);
// => { month: "January", active: false }

This might be useful sometimes in my view, but there was an argument that if we do this, the name findSingle doesn't work well. (by @timreichen) ref: #1166 (comment)

He suggests we should use array.length === 1 ? array[0] : undefined instead of findSingle(array). I partially agree with it, but not 100% convinced.

[majidsajadi]

isn't predicate required in type definition? also according to the description, findSingle finds a single element if matches the given condition. in my opinion, the return in the below example is not what I expect according to the description and type definition.

const bookings = [{ month: 'January', active: false }];
const item = findSingle(bookings);
console.log(item);
// => { month: "January", active: false }

same as native javascript find. the predicate must be provided.

if the method was something like this:

// Returns an random element. the return value matches the given condition if provieded.
declare function random<T>(
  array: readonly T[],
  predicate?: (el: T) => boolean,
): T | undefined 

the default value would make sense to me.

[timreichen]

Just for completeness here the reasons why I find we should remove the default predicate

• the name findSingle corresponds to find and findIndex which have a mandatory predicate.
• these cases all return the same value with the default predicate which does not give the user a clue, if the array is empty, has a single unique value or not:

findSingle([]) // returns undefined
findSingle([undefined]) // returns undefined
findSingle([undefined, undefined]) // returns undefined

[LionC]

Just for completeness here the reasons why I find we should remove the default predicate

• the name findSingle corresponds to find and findIndex which have a mandatory predicate.
• these cases all return the same value with the default predicate which does not give the user a clue, if the array is empty, has a single unique value or not:

findSingle([]) // returns undefined
findSingle([undefined]) // returns undefined
findSingle([undefined, undefined]) // returns undefined

We named the function with the default predicate being there - using the name as an argument afterwards seems very circular.

find functions will return undefined for cases like above for a lot of different "naive" predicates. With TS and a clear signature, I do not see how that causes bugs.

I think this is a discussion about personal style and preference. Neither decision will have a lot of impact. I am not sure if this is important enough to introduce a breaking change to a function right after it was released though @kt3k

I don't think there is a lot to add from my side here, I will leave the decision to the std authorities and focus on adding some more things.

[getspooky]

@kt3k @LionC any new update about this issue.

[kt3k]

I feel the community is slightly in favor of removing the default predicate (timreichen, zhangyuannie, getspooky, majidsajadi seem in favor). So let's merge #1232

[timreichen]

@kt3k I think this issue can be closed since the PR got merged.