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

package-info.java:29-43: The behaviours of the classes of... #1242

Closed
0pdd opened this issue Nov 18, 2019 · 36 comments
Closed

package-info.java:29-43: The behaviours of the classes of... #1242

0pdd opened this issue Nov 18, 2019 · 36 comments

Comments

@0pdd
Copy link
Collaborator

0pdd commented Nov 18, 2019

The puzzle 1184-56133cf3 from #1184 has to be resolved:

* @todo #1184:30min The behaviours of the classes of this package
* are akward because CollectionOf is based on a Scalar while some
* of the tests of the other classes are expecting mutable collections,
* except for Sticky itself.
* This resulted in the use of Sticky in most of the classes of this package
* during the resolution of #1184 to preserve the actual behaviour. Some
* tests of Sticky were also ignored because of this.
* It is as if CollectionOf was once made to be immutable but
* now there is Immutable for that: ask ARC what direction to take and
* apply it to make this whole package consistent. Keep in mind that
* Collection is particular in the way that it is an Iterable and that
* there are no direct implementation of Collection in Java, but only
* sub-interfaces (Set, List, etc) with their own implementation.
* Also don't forget to unignore or modify the tests of Sticky and improve
* the other tests to be clear about the expected behaviour.

The puzzle was created by Victor Noël on 02-Nov-19.

Estimate: 30 minutes, role: DEV.

If you have any technical questions, don't ask me, submit new tickets instead. The task will be "done" when the problem is fixed and the text of the puzzle is removed from the source code. Here is more about PDD and about me.

@0crat
Copy link
Collaborator

0crat commented Nov 18, 2019

@paulodamaso/z please, pay attention to this issue

@0crat 0crat added the scope label Nov 18, 2019
@0crat
Copy link
Collaborator

0crat commented Nov 18, 2019

Job #1242 is now in scope, role is DEV

@0crat
Copy link
Collaborator

0crat commented Nov 18, 2019

The job #1242 assigned to @victornoel/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

@victornoel
Copy link
Collaborator

@paulodamaso can we have some guidance here? The todo explicitly asks about your opinion :)

@victornoel
Copy link
Collaborator

@0crat wait for ARC

@0crat 0crat added the waiting label Nov 24, 2019
@0crat
Copy link
Collaborator

0crat commented Nov 24, 2019

@0crat wait for ARC (here)

@victornoel The impediment for #1242 was registered successfully by @victornoel/z

@paulodamaso
Copy link
Contributor

paulodamaso commented Nov 25, 2019

@victornoel I think that now that we have Immutable, we don't need to make CollectionOf immutable anymore, and we should adapt the code to reflect this.

@victornoel
Copy link
Collaborator

@paulodamaso the question is not so much about making CollectionOf immutable (because it is not actually), but about having it wrapping a Scalar (same with ListOf by the way) and be forced to use Sticky in the other decorators (e.g., Filtered) to be able to have them behave as expected.

I think the source of this problem, is always the same thing that I talked about in other comments:

  • we have decorators for Iterable that really behave as decorators by augmenting the behaviour of Iterable
  • we have decorators for Collection that are actually not decorator, but datastructure that contains the result of the class behaviour
    • for example, filtered behave as a collection of the wrapped collection filtered the FIRST time it is used.

I think we should rely only on Iterable decorators to augment the behaviour of all collections (because they all implement Iterable) and simply:

  1. Remove CollectionOf (see CollectionOf should be deleted #1252)
  2. Have ListOf and SetOf behave as purely datastructure, like LinkedList, or ArrayList, with nicer constructors taking an Iterable`.
  3. Use the following pattern if we need to iterate over a list using as a an example :
new Filtered(s -> s.contains("a"), new ListOf("aa", "ab", "bc")).forEach(System.out::println);
  1. Use the following pattern if we need a new list for example filtered content:
List myList = new ListOf(new Filtered(s -> s.contains("a"), new ListOf("aa", "ab", "bc")));
  1. Remove all Collection, List and Set decorators that becomes useless now (things like Shuffled or Reversed could still make sense and should not be decorators of Iterable or Iterator since they suppose the storage of state to be implemented, which is contrary to those interfaces contract.

@paulodamaso
Copy link
Contributor

@victornoel I think that finally I understood the problem. I really see no point in having a lot of List and Collection decorators, since all of our behavior can sit nicely in the Iterable decorations. But we still have to trim some edges here:

  1. Already put CollectionOf should be deleted #1252 in scope and start removing these bad guys;
  2. I'm thinking about all these data structures and persistent data stuff, and in my opinion I think we should back up from these ideas a little for now. I think that cactoos main objective is to provide extended behaviors to an abstract data structure (hence using the Iterable interface for everyone makes sense) in a elegant (declarative) way. So I don't think that we should bother with the storage of state of these objects, but just with the behavior of these decorations.
  3. I disagree with this. I still want to have an declarative way of iterating a collection in cactoos. We currently have And but as stated in Delete And(Proc, ...) and introduce ForEach(Proc, ...) #1226 it is clearly having more than onre responsibility, so fixing that one should solve the iteration problem;
  4. At the end, all of decorations should look like this I think;
  5. I disagree in not implementing these behaviors as decorators. I think that decoration is one of the core ideas of cactoos and elegant objects doctrine; again, I think that the main focus of cactoos, for now, is to provide these behaviors in an elegant and declarative form.

WDYT?

@victornoel
Copy link
Collaborator

@paulodamaso cool :)

About:
2. Yes, I agree, why not maybe in the future, but if we should proritize, I think we should just remove from scope every ticket pertaining to data structures and collections, and maybe later who knows we can get them back in.
3. Ok, I get your point, it's good with me yes!
5. I'm not clear what would be their behaviour in the end, something like this?

public final class Filtered extends ListEnvelope {
  public Filtered(Predicate filter, List origin) {
    super(new ListOf(new Filtered(filter, origin)));
  }
}

If yes, then I think it's coherent with the rest so I wouldn't be complaining about it ;)

@paulodamaso
Copy link
Contributor

@victornoel Just clearing 5. , maybe I got it wrong. I was thinking in removing all decorations for List and Set, so we should have only ListOf and SetOf in these packages. But as you stated correctly, some of our decorators doesn't make sense for Iterable or Iterator, but just for List (we don't have any decorator for Set). Also, looking through all these classes made me think that the reason they exist maybe is because is too verbose to use ListOf every time we have an Iterable and want to decorate it and make it a list. So your proposal is ok, we don't need to remove them all, but just wrap our Iterables with them correctly.

So, summarizing:

  1. removing collections, starting on CollectionOf should be deleted #1252
  2. let data structures implementation for a future release
  3. iterate using ForEach, to be implemented in Delete And(Proc, ...) and introduce ForEach(Proc, ...) #1226
  4. See 5.
  5. Change all List decorators that have its counterpart in Iterable package to wrap the Iterable one, and keep the ones that does not have siblings in Iterable package implemented directly in List package

Anything to add?

@victornoel
Copy link
Collaborator

@paulodamaso great, that seem cool.

So basically the present issue could start by making ListOf to be an actual implementation of List that can be used normally without this awkward Scalar thing in it (after the PR for #1185 is merged because until then, the awkward Scalar was in the envelope)?

Once this is done we can then remove all those Sticky that are currently needed everywhere (as mentioned in the @todo of this issue) and have a clear simple behaviour (because honestly, currently it is hell, every time you change a little things, every tests break, see for example: #1255 (comment)).

Actually I think list.Sticky could be deleted because it wouldn't make any sense anymore if ListOf is actually just a List (while iterable.Sticky stills makes sense for example).

And then do the same for SetOf.

@victornoel
Copy link
Collaborator

@paulodamaso WDYT?

@paulodamaso
Copy link
Contributor

@victornoel I think we're good, let's follow your suggestions

@victornoel
Copy link
Collaborator

@0crat wait for #1255 to be merged first.

@0crat
Copy link
Collaborator

0crat commented Feb 1, 2020

@0crat wait for #1255 to be merged first. (here)

@victornoel Job #1242 is already on hold

@victornoel
Copy link
Collaborator

victornoel commented Feb 9, 2020

@paulodamaso I did the work in #1290, don't hesitate to double check already to see if I incorrectly approached this for some points discussed above :)

@0pdd
Copy link
Collaborator Author

0pdd commented Feb 11, 2020

@0pdd 14 puzzles #1292, #1293, #1294, #1295, #1296, #1297, #1298, #1299, #1300, #1301, #1302, #1303, #1304, #1305 are still not solved.

@0crat 0crat removed the waiting label Feb 11, 2020
@0crat
Copy link
Collaborator

0crat commented Feb 11, 2020

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

@0crat 0crat removed the scope label Feb 11, 2020
@sereshqua
Copy link

@0crat quality good

@0crat
Copy link
Collaborator

0crat commented Feb 11, 2020

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

@0crat
Copy link
Collaborator

0crat commented Feb 11, 2020

Order was finished, quality is "good": +35 point(s) just awarded to @victornoel/z

@0pdd
Copy link
Collaborator Author

0pdd commented Feb 24, 2020

@0pdd 13 puzzles #1292, #1293, #1295, #1296, #1297, #1298, #1299, #1300, #1301, #1302, #1303, #1304, #1305 are still not solved; solved: #1294.

@0pdd
Copy link
Collaborator Author

0pdd commented Feb 26, 2020

@0pdd 10 puzzles #1292, #1293, #1295, #1298, #1299, #1300, #1301, #1302, #1304, #1305 are still not solved; solved: #1294, #1296, #1297, #1303.

@0pdd
Copy link
Collaborator Author

0pdd commented Mar 9, 2020

@0pdd 10 puzzles #1292, #1295, #1298, #1299, #1300, #1301, #1302, #1304, #1305, #1320 are still not solved; solved: #1293, #1294, #1296, #1297, #1303.

@0pdd
Copy link
Collaborator Author

0pdd commented Mar 15, 2020

@0pdd 10 puzzles #1295, #1298, #1299, #1300, #1301, #1302, #1304, #1305, #1320, #1325 are still not solved; solved: #1292, #1293, #1294, #1296, #1297, #1303.

@0pdd
Copy link
Collaborator Author

0pdd commented Mar 17, 2020

@0pdd 9 puzzles #1295, #1298, #1300, #1301, #1302, #1305, #1320, #1325, #1326 are still not solved; solved: #1292, #1293, #1294, #1296, #1297, #1299, #1303, #1304.

@0pdd
Copy link
Collaborator Author

0pdd commented Mar 29, 2020

@0pdd 8 puzzles #1295, #1298, #1300, #1301, #1302, #1305, #1320, #1325 are still not solved; solved: #1292, #1293, #1294, #1296, #1297, #1299, #1303, #1304, #1326.

@0pdd
Copy link
Collaborator Author

0pdd commented Mar 29, 2020

@0pdd 7 puzzles #1295, #1298, #1300, #1301, #1302, #1305, #1320 are still not solved; solved: #1292, #1293, #1294, #1296, #1297, #1299, #1303, #1304, #1325, #1326.

@0pdd
Copy link
Collaborator Author

0pdd commented Mar 30, 2020

@0pdd 4 puzzles #1295, #1300, #1302, #1320 are still not solved; solved: #1292, #1293, #1294, #1296, #1297, #1298, #1299, #1301, #1303, #1304, #1305, #1325, #1326.

@0pdd
Copy link
Collaborator Author

0pdd commented Apr 1, 2020

@0pdd 4 puzzles #1295, #1302, #1320, #1337 are still not solved; solved: #1292, #1293, #1294, #1296, #1297, #1298, #1299, #1300, #1301, #1303, #1304, #1305, #1325, #1326.

@0pdd
Copy link
Collaborator Author

0pdd commented Apr 3, 2020

@0pdd 2 puzzles #1320, #1337 are still not solved; solved: #1292, #1293, #1294, #1295, #1296, #1297, #1298, #1299, #1300, #1301, #1302, #1303, #1304, #1305, #1325, #1326.

@0pdd
Copy link
Collaborator Author

0pdd commented Apr 23, 2020

@0pdd 2 puzzles #1337, #1349 are still not solved; solved: #1292, #1293, #1294, #1295, #1296, #1297, #1298, #1299, #1300, #1301, #1302, #1303, #1304, #1305, #1320, #1325, #1326.

@0pdd
Copy link
Collaborator Author

0pdd commented Apr 30, 2020

@0pdd the puzzle #1349 is still not solved; solved: #1292, #1293, #1294, #1295, #1296, #1297, #1298, #1299, #1300, #1301, #1302, #1303, #1304, #1305, #1320, #1325, #1326, #1337.

@0pdd
Copy link
Collaborator Author

0pdd commented Sep 13, 2020

@0pdd all 19 puzzles are solved here: #1292, #1293, #1294, #1295, #1296, #1297, #1298, #1299, #1300, #1301, #1302, #1303, #1304, #1305, #1320, #1325, #1326, #1337, #1349.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants