jsor ยท GitHub

@jsor

Copy link Copy Markdown

Member

This fixes a bug where some() never resolves if the input array contains not enough items.

some([1, 2, 3], 4)->then(function() {
    // Never called
});

@jsor

Copy link Copy Markdown

Member Author

clue

Copy link Copy Markdown

Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation seems to suggest otherwise. IMO it makes sense to reject this with a RangeException, but we should probably also add this special case to the README.

Copy link Copy Markdown

Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a note the the README ( e29472f ).

@clue

Copy link Copy Markdown

Member

See remark about documentation for any(), otherwise LGTM ๐Ÿ‘

@jsor

@clue

Copy link Copy Markdown

Member

README now LGTM :)

However, after further consideration, I think this should probably throw a subclass of LogicException rather than RuntimeException. After all this is in fact a logic error which could be detected at "compile time" (i.e. it does not really depend on a runtime decision). What's your view on this?

@jsor

Copy link Copy Markdown

Member Author

Good point. I've used RangeException after discovering this bug while looking through the bluebird code and extended the SPL RangeException (which in turn extends RuntimeException).

I think it would be strange to use the name RangeException and extend LogicException because this would differ from the SPL.

Maybe we should use another name...any suggestions?

joshdifabio

Copy link Copy Markdown

Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would UnderflowException or LengthException, both of which extend LogicException, make sense here?

Edit: just realised that UnderflowException extends RuntimeException.

Copy link Copy Markdown

Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LengthException might be an option...

Copy link Copy Markdown

Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed to LengthException in cd6bb56 .

@jsor

@jsor jsor added this to the v3.0 milestone

Jan 21, 2016

@jsor

Copy link Copy Markdown

Member Author

I'm planning to add this to 3.0.

@jsor jsor removed this from the v3.0 milestone

Mar 25, 2016

@jsor

@jsor

Copy link Copy Markdown

Member Author

Any objections against including this in a 2.4.0 release?

@jsor

@jsor

jsor deleted the some-underflow branch

March 30, 2016 07:22

@cboden

Copy link Copy Markdown

Member

Technically isn't this an API break?

@jsor

Copy link Copy Markdown

Member Author

I think (and iirc we agreed on IRC) that this is a bug fix, because without this fix

some([1, 2, 3], 4)
    ->then(function() {
        // Never called
    })
    ->otherwise(function() {
        // Never called
    });

simply does nothing. It neither calls the fulfillment nor the rejection handler.

@cboden

Copy link Copy Markdown

Member

ah. ๐Ÿ‘ for 2.4 then

Read the original on github.com โ†—