CollectionOf should be deleted

[victornoel]

In the Java Collection hierarchy, Collection is not a concrete class.

I believe it makes no sense to have a class CollectionOf, its behaviour is actually just a duplicate of ListOf in practice.

I propose we simply delete it and let user choose a concrete implementation via ListOf, SetOf or other collections if there are.

[0crat]

@paulodamaso/z please, pay attention to this issue

[paulodamaso]

@victornoel makes sense.

[paulodamaso]

@0crat in

[0crat]

@0crat in (here)

@paulodamaso Job #1252 is now in scope, role is DEV

[0crat]

Bug was reported, see §29: +15 point(s) just awarded to @victornoel/z

[0crat]

The job #1252 assigned to @fanifieiev/z, here is why; the budget is 30 minutes, see §4; please, read §8 and §9; if the task is not clear, read this and this; there will be no monetary reward for this job

[fanifieiev]

@paulodamaso, @victornoel
I think this class CollectionOf should be marked as deprecated for a while until Cactoos-Matchers project removes it from usage within this issue 148
By removing the CollectionOf class right now the build/tests fail as can be seen here.

[victornoel]

@fanifieiev indeed, good point

[paulodamaso]

@fanifieiev We're using version 0.41 in cactoos-matchers, so changing cactoos master branch should not mess things there. That's why I've chosen to first fix this on cacttos and then update cactoos-matchers.

Also, the tests fail because they use the Collection implementation, which has been removed. These tests should be fixed too. Actually, I think that the PR for this issue should start with the removal of the Collection things from all code and tests before we try to delete the files.

[fanifieiev]

@fanifieiev We're using version 0.41 in cactoos-matchers, so changing cactoos master branch should not mess things there. That's why I've chosen to first fix this on cacttos and then update cactoos-matchers.

Also, the tests fail because they use the Collection implementation, which has been removed. These tests should be fixed too. Actually, I think that the PR for this issue should start with the removal of the Collection things from all code and tests before we try to delete the files.

@paulodamaso If I remove the CollectionOf from Cactoos then build fails on Travis(details here). It actually fails because Cactoos-matcher org.llorllale.cactoos.matchers.HasSize uses the CollectionOf.

[paulodamaso]

@fanifieiev So, on top of it all we have a nasty circular dependency between those projects. Let's first remove all CollectionOf usage in the tests, then we take cara bout the rest OK?

[fanifieiev]

@fanifieiev So, on top of it all we have a nasty circular dependency between those projects. Let's first remove all CollectionOf usage in the tests, then we take cara bout the rest OK?

@paulodamaso I have already removed all usages of CollectionOf in Cactoos(except its test only, which tests CollectionOf just for the sake of test coverage). I just marked them as @Deprecated

[fanifieiev]

@0crat wiat for #1256 to be merged

[0crat]

@0crat wiat for #1256 to be merged (here)

@fanifieiev Can't understand "wiat", try one of these:

• assign Assign a performer to this issue
• boost Set a boost for the job
• continue Remove a job's impediment
• hello Just say hello
• in Register this issue in scope (WBS)
• out Close the order and remove this job from scope
• pay Pay a user some extra cash
• quality Review a task
• resign Resign from this issue
• status Check the status of the job
• version Print current version of the bot
• wait Register an impediment for a job

[llorllale]

@victornoel @paulodamaso

I'm not strongly for nor against this proposal, but would just like to note that forcing java.util.List on users would be against the interface segregation principle.

Also:

So, on top of it all we have a nasty circular dependency between those projects.

This was first pointed out by @victornoel. Can anyone quantify the magnitude of the problem?

[victornoel]

@llorllale can you elaborate on

forcing java.util.List on users would be against the interface segregation principle

What do you mean forcing List? What does it mean to create a new Collection when the raison d'être of Collection is to be an abstract super class for all the Java collections?

If you meant that List shouldn't be the default used when someone wants a Collection, then I agree: this is the problem with the current classes in the collection package IMHO: they actually are just List-based implementations. Apart for some of the decorators that actually makes sense such as Synced or Immutable for example.

[llorllale]

@victornoel

can you elaborate on

forcing java.util.List on users would be against the interface segregation principle

Not much to elaborate on: java.util.List is a more expansive set of methods than java.util.Collection. A consumer shouldn't be forced to depend on the method set of List if all they need is `Collection.

What does it mean to create a new Collection when the raison d'être of Collection is to be an abstract super class for all the Java collections?

This is plain false. java.util.Collection is "the root interface in the collection hierarchy". It is not a class, and it's not mean to be merely an abstract type to be extended by subtypes.

If you meant that List shouldn't be the default used when someone wants a Collection, then I agree

We're in agreement, although I'd like to point out again that I'm not strongly arguing one way or the other.

this is the problem with the current classes in the collection package IMHO: they actually are just List-based implementations.

This point does not follow from the previous: no List are being exported from the collection package. Their implementation should generally not matter at all.

[victornoel]

This is plain false. java.util.Collection is "the root interface in the collection hierarchy". It is not a class, and it's not mean to be merely an abstract type to be extended by subtypes.

Let's not be exaggerate ^^ yes, it's not a class, it's an interface, this doesn't change the bottom of what I'm saying. Let me cite the javadoc to clarify what I meant:

The JDK does not provide any direct implementations of this interface: it provides implementations of more specific subinterfaces like Set and List. This interface is typically used to pass collections around and manipulate them where maximum generality is desired.

That's all I meant, I believe it's better to respect the whole precepts of the domain we are trying to fit with. I get your point that yes, you can get a collection without knowing if it's a list or a set, but... when you create it: what is it that you are creating? a list or a set?

I believe it's better to make that decision clearly instead of relying on implicit. Today CollectionOf is implemented with a list, tomorrow it's implemented with a set, and your program does not work anymore.

[fanifieiev]

@victornoel This issue has been completed, PR has been merged. Please take the action.

[llorllale]

@victornoel

I believe it's better to make that decision clearly instead of relying on implicit. Today CollectionOf is implemented with a list, tomorrow it's implemented with a set, and your program does not work anymore.

Point taken.

The use-site declares what dependency it needs. If it declares a dependency on java.util.Collection yet its implementation depends on a concrete behaviour then it should have scoped down its dependency declaration. So: no, point NOT taken.

... but then again, the only thing the use-site need do is declare java.util.Collection as its dependency, ie. no need for CollectionOf. Point taken.

[llorllale]

@victornoel

With the above said, what I'm really interested in is the following:

This was first pointed out by @victornoel. Can anyone quantify the magnitude of the problem?

[fanifieiev]

@victornoel Please take the action. This task has been finalized, we need you close the issue.

[victornoel]

@victornoel This issue has been completed, PR has been merged. Please take the action.

@fanifieiev sorry for the delay, I was away. For the record, if you can, please tell issue reporter as soon as the PR is merged if you want the issue closed fast, I don't follow each and every PR related to issues I opened personally.

[0crat]

The job #1252 is now out of scope

[0crat]

@sereshqua/z please review this job completed by @fanifieiev/z, as in §30; the job will be fully closed and all payments will be made when the quality review is completed

[sereshqua]

@llorllale & @victornoel please make sure you start all your messages with the name of the users they are related to, see: 1, 2, 3, 4

[victornoel]

@sereshqua good points, thanks for the reminder

[llorllale]

@sereshqua fixed mine

[sereshqua]

@0crat quality acceptable

[0crat]

Order was finished, quality is "acceptable": +30 point(s) just awarded to @fanifieiev/z

[0crat]

Quality review completed: +4 point(s) just awarded to @sereshqua/z