why does the compiler complain about this ternary operator: "not a statement"?

[Anil Philip]

Trying to prepare for coding interviews. I don't know why the compiler complains about the ternary operator: "not a statement".

(x > 0) ? positives++: (x == 0) ? zeroes++: negatives++;

I understand I can convert it into an if-else statement.

            arr.stream().forEach(x -> {
                if ((x > 0)) {
                    positives++;
                } else {
                    if ((x == 0)) {
                        zeroes++;
                    } else {
                        negatives++;
                    }
                }
            });

        /*
         * Complete the 'plusMinus' function below.
         *
         * The function accepts INTEGER_ARRAY arr as parameter.
         */
        static int positives = 0;
        static int negatives = 0;
        static int zeroes = 0;


    public static void plusMinus(List<Integer> arr) {
            // Write your code here
            arr.stream().forEach(x -> {(x > 0) ? positives++: (x == 0) ? zeroes++: negatives++;});
            System.out.format("%d.6\n", positives);
            System.out.format("%d.6\n", zeroes);
            System.out.format("%d.6\n", negatives);
    }

[Liutauras Vilda]

Anil Philip wrote:I don't know why the compiler complains about the ternary operator: "not a statement".

Well, the problem is not a ternary operator, but rather an expression not being a statement (expression itself is fine though). It has to be a statement at that point in your code.

So you have to do something with it in order to have a statement, eg.:

int result = (x > 0) ? positives++: (x == 0) ? zeroes++: negatives++;

or

System.out.println((x > 0) ? positives++: (x == 0) ? zeroes++: negatives++);

Expression on its own is not a statement. There are kind of expressions that also can be used as statements if they are followed by semicolon, and I had to look it up in JLS to see how they are officially called, so it is expression statement, eg.:

positives++;

So that ^ can stay as a loose line.

[Anil Philip]

Thank you for your reply.
How would I do it as part of the lambda in the stream?

           arr.stream().forEach(x -> {(x > 0) ? positives++: (x == 0) ? zeroes++: negatives++;});

[Ron McLeod]

Your example will not compile because forEach expects to operate on a Consumer, and a Consumer cannot return a value.

If you wrapped it with something like System.out.println like Liutauras showed before it will also fail to compile because variables in a lambda expression need to be effectively final, and positives, zeroes, and negatives are not.

[Liutauras Vilda]

Anil Philip wrote:How would I do it as part of the lambda in the stream?

Most likely there are many ways to do it, you never said what though. But with a bit of guessing and trying to keep close to what you have, here is a way which first came to my mind:

import java.util.List;

public class Test {

    public static void main(String[] args) {
        List<Integer> numbers = List.of(-5, -4, -3, -2, -1, 0, 0, 1, 2, 3, 4, 5);

        List<Result> results = numbers.stream().map(n -> {
            int positives = 0;
            int zeroes = 0;
            int negatives = 0;
            if (n > 0) {
                positives++;
            } else if (n == 0) {
                zeroes++;
            } else {
                negatives++;
            }
            return new Result(positives, zeroes, negatives);
        }).toList();

        int countOfPositives = results.stream().mapToInt(Result::positives).sum();
        int countOfZeroes = results.stream().mapToInt(Result::zeroes).sum();
        int countOfNegatives = results.stream().mapToInt(Result::negatives).sum();
    }
}

record Result(
        int positives,
        int zeroes,
        int negatives
) {
}

I'm sure there are way more elegant ways of achieving the same.

[Liutauras Vilda]

Here is perhaps slightly less verbose approach:

List<Integer> numbers = List.of(-5, -4, -3, -2, -1, 0, 0, 1, 2, 3, 4, 5);

Function<Integer, Result> f = n -> (n > 0)
        ? new Result(1, 0, 0)
        : (n == 0)
        ? new Result(0, 1, 0)
        : new Result(0, 0, 1);

List<Result> results = numbers.stream().map(f).toList();

I've omitted other parts that are in the earlier post.

[Campbell Ritchie]

Please try not to quote the whole of the preceding post; that doesn't add anything new and the copied text is liable to removal.

Ron McLeod wrote:. . . variables in a lambda expression need to be effectively final . . .

Well, fields are allowed to change, but not parameters and local variables. You probably wouldn't want those three variables to be fields. You could create a local class to do the counting, and that will compile. But if a local class isn't overkill, I don't know what is!
I think that sort of counting is something Streams were not really designed for.
What you could do, however, is to create a Stream<Integer> with the boxed() method.
You can use groupingBy(). For the left argument, use something like Integer#signum(). For the right argument, try another Collector, maybe the one you can get here to do the counting. Note: You might end up with longs for your counts rather than ints. Have a look at Stephans's example here, but that is a rather more complicated situation than yours.
I got it to work on JShell, eventually.

Map<Integer, Long> countMap =
     Arrays.stream(arr) /* Assuming arr is an int[] */
           .boxed()
           .collect(Collectors.groupingBy(
                         Integer::signum,
                         Collectors.counting()));

[Campbell Ritchie]

A few minutes ago, I wrote:. . . You can use groupingBy(). . . .

You use that to get a Collector which you pass to the Stream#collect() method. I lost that part when editing my post.

[Anil Philip]

Ron McLeod wrote:Your example will not compile because forEach expects to operate on a Consumer, and a Consumer cannot return a value.

If you wrapped it with something like System.out.println like Liutauras showed before it will also fail to compile because variables in a lambda expression need to be effectively final, and positives, zeroes, and negatives are not.

In the code sample, they are static instance variables. Here is the full code I wrote. The question is from hacker rank.
Given an array of integers, calculate the ratios of its elements that are positive, negative, and zero. Print the decimal value of each fraction on a new line with  places after the decimal.

package hacker_rank;

import java.io.*;
import java.math.*;
import java.security.*;
import java.text.*;
import java.util.*;
import java.util.concurrent.*;
import java.util.function.*;
import java.util.regex.*;
import java.util.stream.*;
import static java.util.stream.Collectors.joining;
import static java.util.stream.Collectors.toList;

public class PlusMinus {

        /*
         * Complete the 'plusMinus' function below.
         *
         * The function accepts INTEGER_ARRAY arr as parameter.
         */
        static float positives = 0;
        static float negatives = 0;
        static float zeroes = 0;


    public static void plusMinus(List<Integer> arr) {
            // Write your code here
        arr.stream().forEach(x -> (x > 0) ? positives++: (x == 0) ? zeroes++: negatives++);
//            arr.stream().forEach(x -> {
//                if ((x > 0)) {
//                    positives++;
//                } else {
//                    if ((x == 0)) {
//                        zeroes++;
//                    } else {
//                        negatives++;
//                    }
//                }
//            });
            int n = arr.size();
            System.out.format("%.6f\n", positives/n);
            System.out.format("%.6f\n", negatives/n);
            System.out.format("%.6f\n", zeroes/n);
    }

        public static void main(String[] args) throws IOException {
            BufferedReader bufferedReader = new BufferedReader(new InputStreamReader(System.in));

            int n = Integer.parseInt(bufferedReader.readLine().trim());

            List<Integer> arr = Stream.of(bufferedReader.readLine().replaceAll("\\s+$", "").split(" "))
                    .map(Integer::parseInt)
                    .peek(System.out::println)
                    .collect(toList());

            PlusMinus.plusMinus(arr);

            bufferedReader.close();
        }
    }

[Campbell Ritchie]

Anil Philip wrote:. . . static instance variables. . . .

Fields can be either static type or instance type (sometimes called non‑static), but not both. Those are static fields. Didn't HackerRank tell you that static variables are usually a bad idea?
There are quite a lot of things I don't like about that code.

Calling a List arr.

Reading from the keyboard would be much easier with a Scanner object.

Scanner inScan =
      new Scanner(System.in); // Don't close this Scanner
List<Integer> numbers =
      inScan.tokens()
            .filter(s -> newScanner(s).hasNextInt())
            .map(Integer::valueOf)
            .collect(Collectors.toList());

/* Alternative: lets you to specify runtime type of List */ Scanner inScan =      new Scanner(System.in); // Don't close this Scanner List<Integer> numbers =      inScan.tokens()            .filter(s -> newScanner(s).hasNextInt())            .map(Integer::valueOf)            .collect(ArrayList::new, List::add, List::addAll);

Use of the float datatype for counting. Use of the float datatype for anything Use integer datatypes for counting.

Making everything static, except in functional programming. This code isn't functional.

[Campbell Ritchie]

The HackerRank link says to calculate the proportions, which you can do with something like 1.0 * positives / total. The 1.0 * part converts the numbers to doubles.

[Anil Philip]

Campbell Ritchie wrote:

Anil Philip wrote:. . . static instance variables. . . .

Fields can be either static type or instance type (sometimes called non‑static), but not both. Those are static fields. Didn't HackerRank tell you that static variables are usually a bad idea?

oops, I meant static member variables.
To access them from the lambda and also update them, they have to be member variables (instance or class).
Also, we are required to fill in only the plusMinus() method - nothing else. The reading input is provided by them.

[Liutauras Vilda]

Anil Philip wrote:In the code sample, they are static instance variables.

I registered to that platform and I couldn't find them. You are asked to implement plusMinus() function that returns nothing, it should just print result as requested. I've passed the tests with my almost original code. I didn't know about ratios.

[Anil Philip]

Liutauras Vilda wrote:

Anil Philip wrote:In the code sample, they are static instance variables.

I registered to that platform and I couldn't find them. You are asked to implement plusMinus() function that returns nothing, it should just print result as requested. I've passed the tests with my almost original code. I didn't know about ratios.

Are you able to see Plus Minus in the challenge over here. 1 week interview prep kit?
My code works when I use the equivalent if-else (see commented out portion of code in my last post) instead of the expression.
But they have something called "hidden tests" that have to be unlocked, no idea what they are (probably pay them some money - buy them some "pie" ), and it did not pass them all.

[Anil Philip]

Campbell Ritchie wrote:The HackerRank link says to calculate the proportions, which you can do with something like 1.0 * positives / total. The 1.0 * part converts the numbers to doubles.

But if you start with a float, it is done automatically.

[Carey Brown]

@Anil,

            int n = arr.size();
            System.out.format("%.6f\n", positives/n);
            System.out.format("%.6f\n", negatives/n);
            System.out.format("%.6f\n", zeroes/n);

You made "positives" (etc) float in order to have a floating point result from the divide. (Almost) Never use "float", use "double" instead it is the internal floating point default type and is more efficient. "float" should only be used when interfacing with legacy code.

Additionally,  making those three variables float incurs some overhead for every increment (++) operation, change them to long. Then change  "n" to "double". Now the results of the divisions  will be floating point without the increment penalty.

[Paul Clapham]

I decided to see if you could use a method reference to rehabilitate your code. And yes, you can. Here's my revised version:

	public static void main(String[] args) {
		plusMinus(List.of(2,12,-4,0,0,13));
	}

	static int positives = 0;
    static int negatives = 0;
    static int zeroes = 0;

    private static void plusMinus(List<Integer> arr) {
        arr.stream().forEach(Test::incrementValues);
        System.out.format("%d.6\n", positives);
        System.out.format("%d.6\n", zeroes);
        System.out.format("%d.6\n", negatives);
    }
    private static void incrementValues(int x) {
    	int whatever = (x > 0) ? positives++: (x == 0) ? zeroes++: negatives++;
    }

Note the unnecessary "whatever" variable there, it is actually necessary to allow the code to compile, for the reasons already discussed. You'll get a warning from the compiler, though. But at least it works in this context and it's a minimal change from your original try. And besides, method references are pure functional programming.

As you see, even static methods can be used as the target of a method reference. (I assumed that your class was named "Test".) That's a bit in bad taste, though, it's generally recommended to eschew static methods as much as possible.

[Carey Brown]

There's nothing in Hackerrank that requires that streams or lambdas be used.

[Anil Philip]

Carey Brown wrote:@Anil,

            int n = arr.size();
            System.out.format("%.6f\n", positives/n);
            System.out.format("%.6f\n", negatives/n);
            System.out.format("%.6f\n", zeroes/n);

You made "positives" (etc) float in order to have a floating point result from the divide. (Almost) Never use "float", use "double" instead it is the internal floating point default type and is more efficient. "float" should only be used when interfacing with legacy code.

Additionally,  making those three variables float incurs some overhead for every increment (++) operation, change them to long. Then change  "n" to "double". Now the results of the divisions  will be floating point without the increment penalty.

Good points. Thanks!

[Anil Philip]

Paul Clapham wrote:I decided to see if you could use a method reference to rehabilitate your code. And yes, you can. Here's my revised version:

	public static void main(String[] args) {
		plusMinus(List.of(2,12,-4,0,0,13));
	}

	static int positives = 0;
    static int negatives = 0;
    static int zeroes = 0;

    private static void plusMinus(List<Integer> arr) {
        arr.stream().forEach(Test::incrementValues);
        System.out.format("%d.6\n", positives);
        System.out.format("%d.6\n", zeroes);
        System.out.format("%d.6\n", negatives);
    }
    private static void incrementValues(int x) {
    	int whatever = (x > 0) ? positives++: (x == 0) ? zeroes++: negatives++;
    }

Note the unnecessary "whatever" variable there, it is actually necessary to allow the code to compile, for the reasons already discussed. You'll get a warning from the compiler, though. But at least it works in this context and it's a minimal change from your original try. And besides, method references are pure functional programming.

As you see, even static methods can be used as the target of a method reference. (I assumed that your class was named "Test".) That's a bit in bad taste, though, it's generally recommended to eschew static methods as much as possible.

Thank you sir! This is the solution I was looking for. To be able to use the original expression.

[Paul Clapham]

Carey Brown wrote:There's nothing in Hackerrank that requires that streams or lambdas be used.

I'm still in favour of experimenting with functional programming, though.

[Anil Philip]

Carey Brown wrote:There's nothing in Hackerrank that requires that streams or lambdas be used.

It's a good exercise to use streams and lambdas because I think that they make for less complex code and fewer bugs. I never thought I would say this about streams and lambdas! they took a while to get used to.

[Carey Brown]

    private static void incrementValues(int x) {
        // until some maintainer removes  "whatever" because there
        // are no comments
    	int whatever = (x > 0) ? positives++: (x == 0) ? zeroes++: negatives++;
    }

[Paul Clapham]

Carey Brown wrote:

    private static void incrementValues(int x) {
        // until some maintainer removes  "whatever" because there
        // are no comments
    	int whatever = (x > 0) ? positives++: (x == 0) ? zeroes++: negatives++;
    }

Yes, I didn't put in @SuppressWarnings to take care of that. But the maintainer who removed the declaration part would then be facing the compiler error which was the topic of this thread. They could ask about it here, we're always ready to help.

[Carey Brown]

I was thinking of something more preemptive.

private static void incrementValues(int x) {
    // WARNING: Do not remove "int whatever =" or it won't compile.
    // It is required to convert the expression into a statement.
    int whatever = (x > 0) ? positives++: (x == 0) ? zeroes++: negatives++;
}

[Campbell Ritchie]

Anil Philip wrote:. . . if you start with a float, . . .

As I said, do your counting with integer types. I don't like to see floats anywhere because of their poor precision.

[Campbell Ritchie]

Carey Brown wrote:There's nothing in Hackerrank that requires that streams or lambdas be used.

Nor @SuppressWarnings. That template looks like pre‑Java5 code

[Campbell Ritchie]

Anil Philip wrote:. . . Are you able to see Plus Minus . . . ? . . .

Not without logging on to that site, I am afraid. Just as well you supplied a copy for us.

[Anil Philip]

Ron McLeod wrote:Your example will not compile because forEach expects to operate on a Consumer, and a Consumer cannot return a value.
If you wrapped it with something like System.out.println like Liutauras showed before it will also fail to compile because variables in a lambda expression need to be effectively final, and positives, zeroes, and negatives are not.

I did not know that you must not use an expression that returns a value in a Consumer lambda. Thanks.
Someone else mentioned (can't find their post) using an assignment to swallow the return value and get around it.
I found that this works:

        arr.stream().forEach(x -> {int i = (x > 0) ? positives++: (x == 0) ? zeroes++: negatives++;});

        static int positives = 0;
        static int negatives = 0;
        static int zeroes = 0;


    public static void plusMinus(List<Integer> arr) {
            // Write your code here
        arr.stream().forEach(x -> {int i = (x > 0) ? positives++: (x == 0) ? zeroes++: negatives++;});
            double n = arr.size();
            System.out.format("%.6f\n", positives/n);
            System.out.format("%.6f\n", negatives/n);
            System.out.format("%.6f\n", zeroes/n);
    }

[Stephan van Hulst]

Great, you made it work. Now, never write code like that ever again.

Either use a simple for-loop, or use a collector.

[Campbell Ritchie]

Last night, I wrote:. . . Reading from the keyboard would be much easier with a Scanner object. . . .

Did anybody notice the logic error in what I posted?

[Anil Philip]

Stephan van Hulst wrote:Great, you made it work. Now, never write code like that ever again.

Either use a simple for-loop, or use a collector.

Huh? I am quite proud of what I wrote.

[Campbell Ritchie]

Anil Philip wrote:. . . I did not know that you must not use an expression that returns a value in a Consumer lambda.

For that sort of thing, always look in the documentation. Never trry to learn what the documentation says; you won't remember anything used less frequently than Math.max() or System.out.printf(). If you look at the documentation for Consumer, you will find it has one abstract method, and that link will show you it has void instead of a return type. The fact that it says void means it can't return anything.

Thanks. . . .

That's a pleasure

[Campbell Ritchie]

A minute ago, I wrote:. . . Never try to learn what the documentation says . . .

Except that, if you sit a certification exam, the exam syllabus will expect you to learn what the documentation says.

[Liutauras Vilda]

Carey Brown wrote:I was thinking of something more preemptive.

private static void incrementValues(int x) {
    // WARNING: Do not remove "int whatever =" or it won't compile.
    // It is required to convert the expression into a statement.
    int whatever = (x > 0) ? positives++: (x == 0) ? zeroes++: negatives++;
}

To write an unnecessary stuff to bypass compiler doesn't sound right, primarily semantically, that introduces a necessity to write comment out of nowhere.
I'd not write any comment here, and would not use this construct at all for the same reason.

What I mentioned earlier to Anil, I registered yesterday to hackerank to check the problem at hand and I couldn't see in the provided template any static variables. So I'm not sure where they came from. I solved that problem by using collector to collect results and then did printing them out (didn't use any static variables), and I'd suggest Anil to refactor his code too.

Anil, you mentioned you are doing those exercise to prepare for an interview. Now, those interviews may differ in many ways, some of those are just to solve problem and bypass some auto grader tool. Some coding challenges might be, that in addition to passing an auto grader, someone is going to review your code for its elegancy. Some interviews might be, that they don't care much about solution end-to-end, but rather a journey to it, how you decompose problem, what's your thinking process behind, how you name variables, whether you solve it in a disciplined manner and similar.. So try to be a best version of yourself when solving them, not just get unit tests to pass.

[Carey Brown]

Anil Philip wrote:

Stephan van Hulst wrote:Great, you made it work. Now, never write code like that ever again.
Either use a simple for-loop, or use a collector.

Huh? I am quite proud of what I wrote.

Ternary expressions weren't meant to be used to cause side effects but you managed to beat it into submission to get it to work. Now, what about the poor developer that comes after you that has to maintain it? He'd have to learn everything you just learned in  order to properly read the code.

Aside from the non-intuitive constructs, I object to using static variables as accumulators. If you write it with loops then the accumulators can be local variables.

[Anil Philip]

Liutauras Vilda wrote:
What I mentioned earlier to Anil, I registered yesterday to hackerank to check the problem at hand and I couldn't see in the provided template any static variables. So I'm not sure where they came from. I solved that problem by using collector to collect results and then did printing them out (didn't use any static variables), and I'd suggest Anil to refactor his code too.
.

Can you please share your code using collectors? I would like to learn how to do it that way.
The method provided by HackerRank was static and so I had to use static member variables.

[Carey Brown]

Anil Philip wrote:The method provided by HackerRank was static and so I had to use static member variables.

Not true. A simple loop, for example, would not have needed static member variables. It could use local variables.

[Campbell Ritchie]

Anil Philip wrote:. . . Can you please share your code using collectors? . . .

I already have.

The method provided by HackerRank was static and so I had to use static member variables.

You could have used local variables as long as you didn't use them in a λ or anonymous class. I think the compiler error message about “non‑static” is less than crystal‑clear, and confuses people into making things static that oughtn't to be static.
* * * * * * * * * * * * *
On a different note, you started a very interesting discussion here

[Anil Philip]

Carey Brown wrote:

Anil Philip wrote:The method provided by HackerRank was static and so I had to use static member variables.

Not true. A simple loop, for example, would not have needed static member variables. It could use local variables.

I do not wish to use loops. I want to use lambdas and streams to improve my understanding and facility with them.