:mortar_board: :technologist: :lipstick: Bag implementations --- Remove method returns `false`

[jameshughes89]

Related Issues or PRs

899

What

Have it such that the bag implementations return false when the element does not exist within the bag.

Why

Consistency with java collections

How

This one has a little more nuance since before the remove could throw two exceptions: one for when the bag was empty and one for when the element did not exist. I opted to just return false in both cases. This made the method slightly simpler too.

This got kinda' weird though since, for example, in the sorted bag, exceptions are still thrown if the bag is empty when calling remove first/last. My justification for this is, the general remove returns a boolean, so the boolean tells us what we need to know on the success/fail of the method call. For the remove first/last, they return an actual element, so the exception still makes sense here (sure I could return null if its empty, but that seems bad).

Testing

:+1:

[twentylemon]

This got kinda' weird though since, for example, in the sorted bag, exceptions are still thrown if the bag is empty when calling remove first/last. My justification for this is, the general remove returns a boolean, so the boolean tells us what we need to know on the success/fail of the method call. For the remove first/last, they return an actual element, so the exception still makes sense here (sure I could return null if its empty, but that seems bad).

I would tend to argue the same way. removeFirst expects there to be a first, and should remove it. It's essentially the same as .head from scala collections.