amaembo / streamex

Enhancing Java Stream API
Apache License 2.0
2.18k stars 249 forks source link

handle throwable functions in methods of StreamEx #275

Closed msoares80 closed 8 months ago

msoares80 commented 8 months ago

Hi ! I would to push 2 waves of new functionnalities for the streamEx library. My actual push needs a lot more of work to be totally validated but i would like to know if the idea i would like to push may interest you and i would like to be sure that my final completed version will be merged in master with your validation when everything will be ok for you.

Here, my push contains only elements of the first wave. If you are interested, i could push a new branch with elements of second wave that i have already started to implement.

The idea of my first wave is to introduce Throwable functions interface in methods of Stream. These functional interfaces are likely the same as FunctionalInteface java like Function, Consumer, Supplier except the apply method accept to throw an Exception. Then, all methods of Stream like, map, flatMap, [...] that take a Function, Consumer, Supplier in parameter will have duplicated methods like this :

In final these 2 new methods on each method of Stream will call their conventional equivalent : mapThrowing calls tryMap that calls map. The difference is not in the functionality of the map but that the method can accept Throwable functions with 2 options :

In the StreamExInternalTest class, i have created some tests to explain in details the interest of this idea with commentary to help to explain. Also, to quickly increase my percentage of code cover, i have replaced some call methods of your tests to use the throwing functions instead the conventional. In this way, you can also see that the new functions accepts methods that not throwing too.

If the idea please you but you are not agree with some syntax or any thing else, it is not a problem with me : you can propose any changes. I will adapt. In this way, i have created a package-info.java in functionex package more like a free discussion for now to talk about some conventions of naming. And, if the idea please you, i will not say no for some help :). Above all, in javadocs because my english is shaky.

The following actions is to continue to create new methods for other StreamEx functions that I skip for now.

In second, I would like too, to move the package functionex in another lib and streamEx will use it. For the second wave that i would like to propose, i need Throwable Functions classes but StreamEx is not needed (for now, but maybe, i'm wrong :) ). It would be interesting to not oblige users to integrate all the classes of my second wave if not needed in their application. But we will can talk about that later.

Thank you for giving me your attention.

amaembo commented 8 months ago

Hello!

I'm sorry to say, but such a pull-request cannot be accepted into StreamEx project for a number of reasons. Of course, I considered working around the checked exception issues, there were big discussions on this topic around years 2014-2015 on StackOverflow, OpenJDK mailing lists, and there were a number of alternative projects enhancing Stream API that tried to solve this problem. I concluded that all the possible solutions are unsatisfactory, and decided that this problem is out of scope of my library. In particular:

   try {
      somethingThatDoesSneakyThrow();
   }
   catch(Exception ex) {
       if (ex instanceof MyCheckedException) {
           // handle it
       }
   }

Of course, static analyzers rightfully will scream on this code, so you'll have to suppress their warnings as well. This might look as a smart move, but it's not the way people write maintainable software in Java. Sorry, but we are not Lombok.

I hope that you understand. Of course, you are free to maintain your own fork of StreamEx under the terms of Apache 2.0 License and add your functionality there.

In general, it's a good idea to create an issue first to discuss your ideas on how to enhance an open-source project before doing an actual work.

msoares80 commented 8 months ago

Hi Amaembo,

Thank you for taking time to answer me. I'm not used to propose new functionality in libraries and i don't know the good way to do that. Maybe, create an issue should be better to avoid lose too much time. It is not serious. At least, we have a support on which we can talk with some examples.

I read your answer and i understand that you don't want to integrate this functionality in your library and i will not push too long. Nevertheless, i would like to have precision in some of your remarks and explain my point of view for some parts.

  1. For example, for the most important point : the case of xyzThrowing methods. For you, it is completely broken because methods xyzThrowing don't throw effectively any Exceptions. These exceptions will be thrown after in a terminal operation. But, in my point of view that you have right to not accept, the throws declaration is just a declaration that the exception can be thrown during the process and in the reality of fact, this is true. Here, we are using monads that declare actions to do in a terminal operation, then for me, it is not an error to declare exceptions that could be thrown during the terminal operation. It is the same as you are telling me that you can't pass a lambda function in a method because this lambda will not be played here but in a terminal operation. For me, we declare a lambda function and if this lambda can throws exception, it is normal for me to declare it in the same time. It is just my point of view.

  2. I have not understand the first case with tryXyz methods because the sneaky throw converts the Exception as an RuntimeException without losing his real type. And, in the present, we don't have a lot of choice if my lambda have to throw an Exception :

    • Either, i have to handle it directly in my lambda but it not everytime possible
    • Either, i have to throw a RuntimeException. The consequences, compared to what I have seen in several projects where i work, they are not using monads and Stream api or they convert all domain exception in RuntimeException. And about your example of code you are showing, it was exactly the goal of xyzThrowing method's use case to avoid that.
  3. In the part you are talking about that these new methods will concern 1% of cases. I am not agree with that. What i am seeing in projects where i worked, is developers do compromise like i have told before. But convert an important DomainException in a RuntimeException is not such a good idea. After that, they can still wrap their methods in an unchecked() method that wraps exception in a sneaky throw but it seems not to be a well-knwon functionality.

  4. In your fourth point, it is ok. I totally understand that StreamEx should depend on new FunctionInterface not managed by Java. Even if in the utilisation, we can see that mapThrowing() and map() can thake exactly the same lambda and there is no difference. This idea was more interesting if you should accept this point. But, if it is not the case : it was just a try.

  5. For the Tuple, i don't think it is a so extremely bad programming because my Tuple object herits of Map.Entry. After that, i understand that it will be better to write all the API with Tuple or Map.Entry but not a mix. And i understand that your library is near of Java class and create a new class that does the same thing is not very usefull. Of course, i would delete this class if necessary. I have created it to simplify some tests :).

Ok, thank you for your time. Maybe, i should propose my modifications in libraries like io.vavr because they have already coded new FunctionalInterfaces and they have rewritten from zero a new library Collection with a minimal correlation with Java API. It seems that they are ok with this problem in their library. However, I liked your library because you have coded the EntryStream not like vavr, but it can be also a new proposition. But, of course, before create a fork, i will create a issue :)