3
// the results can be anything, errors always start with 'ERROR'
List<String> results= { 
    "Success 100 - Operation worked John", 
    "Success 100 - It also worked for Harry", 
    "ERROR 4514 for Sally. It's always Sally." 
}

// I want this to output something like 
//     warn: There were errors
//     warn: ERROR 4514 for Sally. It's always Sally.
//
// in the case there are no ERROR's I want no warn:'s
results.stream()
    .filter( name->name.startsWith( "ERROR" ) )
    .DO_THIS_IF_NOT_EMPTY( ()-> LOG.warn( "There were errors"; )
    .forEach( error -> { LOG.warn( "ERROR: " + error }

The DO_THIS_IF_NOT_EMPTY is wishful. I don't see an obvious elegant way to do this with java streams. Can anyone think of a good one?

Holger
  • 285,553
  • 42
  • 434
  • 765
  • Within the stream all operations should be *stateless*. So just filter the imput with the stream and do the output separately. – Timothy Truckle Mar 14 '17 at 15:32
  • 1
    Typical. Women get blamed for everything :-) – Sean Patrick Floyd Mar 14 '17 at 15:33
  • Streams have an overhead cost. There was a post a couple of weeks back that had good solid numbers about this overhead, stream size matters. It seems that you are saving a couple of lines of code at the expense of performance – bichito Mar 14 '17 at 15:35
  • 1
    @efekctive can you find that post plz? – Eugene Mar 14 '17 at 15:36
  • 1
    @Eugene don't know if that's the one, but I recently benchmarked streams against non-streams in this answer: http://stackoverflow.com/a/42627322/342852 . TL;DR: Streams are efficient when dealing with tens of thousands of items or more. For smaller numbers, imperative loops perform better – Sean Patrick Floyd Mar 14 '17 at 15:39
  • http://stackoverflow.com/questions/42375003/why-lambda-intstream-anymatch-is-10-slower-than-naive-implementation#comment71904012_42375003 Read the answer. – bichito Mar 14 '17 at 15:47
  • @SeanPatrickFloyd It is a different one but same conclusion. The word Stream defines the API. It is not about saving lines of code, it is about process significant amounts of data – bichito Mar 14 '17 at 16:02
  • 2
    @efekctive true, but it's also about a less error-prone programming paradigm, where side effects are discouraged – Sean Patrick Floyd Mar 14 '17 at 16:05
  • @efekctive interesting, I thought point of them was that they're functional and you can leave the compiler to figure out optimal strategies for arranging the pieces and so in. –  Mar 14 '17 at 16:08
  • The objection I have is that if one repeats these saving lines strategies, the overall performance will dive. So I like measure both before settling on one – bichito Mar 14 '17 at 16:18
  • It's really not about saving lines, it's about making sense and presenting code in a way that won't be misunderstood. –  Mar 14 '17 at 16:20
  • 4
    `LOG.warn("There were errors")` looks like you are trying to do *formatting* at the entirely wrong end of the logging framework, as this isn’t a real logging message, but a title to the follow-up messages. Since there is no guaranty that these messages appear adjacent in a log file, this doesn’t make sense. Instead, you should use a log analyzing tool, being able to gather log messages from the same source happening at almost the same time, regardless of where they appear in the log file. Then, the fact that you see one or more error logs is enough to recognize that “There were errors”… – Holger Mar 14 '17 at 17:06
  • 1
    @efekctive The OP didn't ask about performance. In the absence of specific performance requirements (99% of the time or more, the obvious code is fast enough), one should focus exclusively on writing clear, maintainable code, and only address performance if a) there are performance requirements, b) there are performance metrics, and c) the metrics say you don't meet requirements. Interjecting with "but .. performance!", like here, is usually an unconstructive diversion. Your assumption that this is about "line saving" is incorrect; it's about clarity. Clear code is more likely to be correct. – Brian Goetz Mar 14 '17 at 19:14
  • That is your opinion and I respect it. – bichito Mar 14 '17 at 19:37
  • @Holger the q's not about Logging frameworks I threw that in as simple example most people would relate to. Picture an email being formed to send out to customers based on orders if you like, or a JFrame you only create if there are contents to put inside it if that helps. The Q is about java 1.8 streams –  Mar 15 '17 at 12:00

5 Answers5

3

In this case, there is no intrinsic Stream operation, so falling back to use an Iterator is the simplest solution:

Iterator<String> it = results.stream()
    .filter(name -> name.startsWith( "ERROR" ))
    .iterator();
if(it.hasNext()) {
    LOG.warn("There were errors");
    it.forEachRemaining(error -> LOG.warn("ERROR: " + error));
}

That said, there is no sense in logging a message like “There were errors" that doesn’t provide any context. Especially when you are going to produce a meaningful log record right afterwards, which provides the actual message and implies that there were errors..

Apparently, this is supposed to be a title, in other words, you are trying to do log file formatting at the entirely wrong end of the logging framework.

Since there is no guaranty that these messages appear adjacent in a log file, just consider that there might be other log messages from other threads or subsystems, this doesn’t make sense.

Instead, you should use a log analyzing tool, which is able to filter the log file to find records from the same origin, perhaps looking for those that were filed shortly after each other from the same thread, which makes you independent from their actual location in the log file.

You know that “There were errors” when the tool shows you at least one matching record…

Holger
  • 285,553
  • 42
  • 434
  • 765
  • This is a very simplified example for brevity of problem description. Moving the goalposts answers by saying "There were errors" isn't a semantically helpful log message is missing the *** point completely that this is an example shrunk down from a real much bigger problem that you wouldn't expect people to take to to read the whole of Please assume that people you are replying to have more than a grain of intelligence in approaching their problem and that I'm not really handling error codes written as Strings for Harry, John and Sarah and logging it -.- –  Mar 15 '17 at 11:30
  • Then, `LOG.warn` is a badly chosen placeholder, as logging a *warning* message clearly has a certain semantic differing from arbitrary code. Anyway, I started my answer with a *solution* to the general problem. You can ignore the second half, just like you want me to ignore the `LOG.warn` statements… – Holger Mar 15 '17 at 11:35
  • What the question asked was clear. I asked how to fix my sink, you answered "you don't need to wash your hands". That reasoning style is not helpful. This isn't about semantic differences and the problem was never about loggers, it's clear the question is about java 1.8 and how to structure streams - I provided a logger as a simple example most people would relate to. –  Mar 15 '17 at 11:43
  • As said, there is a **solution** at the beginning of the answer, using `Iterator`s. It is *you* making a senseless discussion about something that you are saying, is irrelevant. If you want reader to ignore your use of loggers, why can’t *you* ignore my statement regarding loggers? Can you use the `Iterator` based solution? If yes, fine. If no, good luck finding a better one. If you think, the logger part is too distracting, improve your question first. – Holger Mar 15 '17 at 11:46
  • You need to get over loggers. *"How in Java 1.8 Streams to perform logic before a foreach only if there is at least one element"* is not ambiguous, and not about logging frameworks. Make it System.out.println in your head if it helps you move beyond this. –  Mar 15 '17 at 12:06
  • 1
    Why are you wasting my time with this pointless discussion? Again, **there is a solution at the beginning of the answer**. There is a follow-up paragraph regarding your actual code example, make it empty in your head if it helps you move beyond this. – Holger Mar 15 '17 at 12:09
2

Filter the list :

List<String> errors = names.stream()
    .filter(name -> name.startsWith("ERROR"))
    .collect(Collectors.toList());

And if not empty, print your static line and then do forEach

if (!errors.isEmpty()) {
    LOG.warn("There were errors");
    errors.forEach(error -> LOG.warn("ERROR: " + error));
}
4castle
  • 32,613
  • 11
  • 69
  • 106
Abubakkar
  • 15,488
  • 8
  • 55
  • 83
  • instead of the `.size()` check, the more idiomatic way is to check `if(!errors.isEmpty())` – Sean Patrick Floyd Mar 14 '17 at 15:35
  • sure, but collecting then doing stuff then carrying on (which is what current code does incidentally) kind of loses the spirit of streams I feel and when you have lots of streams loses readability somewhat –  Mar 14 '17 at 16:11
  • The streams are designed such that any terminal operation would lead to evaluation of the stream. In your case, when you want to do something if the stream is not empty sounds a terminal operation to me. – Abubakkar Mar 14 '17 at 16:15
  • 2
    Alternately, if you don't want to materialize the collection, you can use the `iterator()` terminal op, and then walk the iterator afterwards. – Brian Goetz Mar 14 '17 at 19:09
1

Here another, more compact, solution:

Given:

List<String> names = Arrays.asList(
        "Success 100 - Operation worked John",
        "Success 100 - It also worked for Harry",
        "ERROR 4514 for Sally. It's always Sally.");

Predicate<String> IS_ERROR = name -> name.startsWith("ERROR");

Compact Solution:

if (names.stream().anyMatch(IS_ERROR)) {
    LOG.warn("There were errors");
    names.stream().filter(IS_ERROR).forEach(LOG::warn);
}
Harmlezz
  • 7,972
  • 27
  • 35
  • interesting. I probably like this the most of all responses. It uses a double stream obviously, but it's fairly clear. –  Mar 15 '17 at 11:33
  • My solution is not optimized for performance or resources, but for readability. It reflects very clearly what you stated in your requirements: if the messages do contain errors, than do log them. As long as you do need best performance, but care about readability and maintainability of your code base, I would suggest to go with the most readable and least complex solution, as long as you are aware of the tradeoff you made. – Harmlezz Mar 15 '17 at 11:53
  • yeah exactly, like I said, I think this is the bests and looks most like what I wished I would see –  Mar 15 '17 at 11:57
0

You could keep a final AtomicBoolean to keep track of whether this is the first or not.

static String[] results = {
        "Success 100 - Operation worked John",
        "Success 100 - It also worked for Harry",
        "ERROR 4514 for Sally. It's always Sally."
};

public void test() {
    List<String> names = Arrays.asList(results);
    final AtomicBoolean first = new AtomicBoolean(true);
    names.stream()
            .filter(name -> name.startsWith("ERROR"))
            .forEach(error -> {
                if (first.getAndSet(false)) {
                    System.out.println("There were errors ");
                }

                System.out.println("ERROR: " + error);
            });
}

IMHO @abubakkar's solution is a better one.

Community
  • 1
  • 1
OldCurmudgeon
  • 64,482
  • 16
  • 119
  • 213
  • Okay but this isn't elegant and it needs studying to understand what it does –  Mar 14 '17 at 15:44
  • @Timo I think you edited the wrong post. This is the answer section. – 4castle Mar 14 '17 at 16:54
  • This is comment section of OldCurmudgeon answer is it not? This is a comment for OldCurmudgeon's answer... –  Mar 15 '17 at 11:27
-1

You could try something like

results.stream().filter(s -> s.startsWith("ERROR")).forEach(s -> System.out.println("Warning found match: "+ s));
justMe
  • 2,200
  • 16
  • 20
  • 1
    this isn't really the point of the question which is the pre "There were errors" line –  Mar 14 '17 at 16:10
  • Right, my bad, I thought you wanted to log something if an element starts with ERROR – justMe Mar 15 '17 at 09:38