0

I have an ArrayList of objects, the object is an instruction class that has the following variables: toDelete, toUse, ID and executed. To have my program finish, I need to return true only when all members of the ArrayList's executed boolean return true.

public boolean getComplete() {
    for (int i = 0; i < instructionArea.size(); i++) {
        if (instructionArea.get(i).getExecuted() == false) {
            instructionsCompleted = false;
            return instructionsCompleted;
        } else
            instructionsCompleted = true;
    }
    System.out.println(instructionsCompleted);
    return instructionsCompleted;
}

This is my work so far, but I know it is wrong, the program does not execute properly. Any help would be appreciated, thank you.

Thank you responses, I actually realized I had some errors in my code elsewhere that was causing this to not work. Instructions were set to "executed" before they were supposed to. Oh well! Thanks everyone

  • 3
    Can you clarify what you mean by "the program does not execute properly"? Specifically, what behavior are seeing and how does that contrast with what you are hoping/expecting? – thehale May 29 '21 at 21:57
  • From a theoretical point, the algorithm looks correct on a glance (i.e. the method will return `true` iff. the method `getExecuted()` of all objects in `instructionArea` is returns `true` and `false` otherwise). I second @jhale1805's request for a [MRE]. – Turing85 May 29 '21 at 22:00

4 Answers4

0

Your algorithm looks fine, assuming that the variables instructionArea and instructionsCompleted are defined elsewhere and visible to this function. That said, here are two enhancements that may make the code easier to read (and therefore debug).

Note that I restructured the function to receive an ArrayList of Booleans as a parameter. In your case, you'd replace Boolean with your Instruction object and bool with instruction.getExecuted()

For-each Loop

Since you are using an ArrayList, it's a little more efficient to use a for-each loop instead of indexing. Consider a function like the following.

public boolean areAllTrue(ArrayList<Boolean> bools) {
    for (Boolean bool : bools) {
        if (!bool) return false;  // Return false if any element isn't true.
    }
    return true;
}

Java's Streaming API

You can also one-line this function using Java's Streaming API.

public boolean areAllTrue(ArrayList<Boolean> bools) {
    return bools.stream().allMatch(bool -> bool == true);
}
thehale
  • 913
  • 10
  • 18
0

Or you do a reverse to save one more line :) Its stream API in the style like; "Is the any Match, that has not been executed)

public boolean getComplete() {
    return !instructionArea.stream().anyMatch(i -> !i.getExecuted());
}
0
public boolean getComplete() {
    return instructionArea.stream().allMatch(InstructionArea::getExecuted);
}

Your logic may be expressed as return true if all elements return true from getExecuted() and this code reflects that exactly.

It also uses a method reference (instead of a lambda), which IMHO makes for more elegant code.

Bohemian
  • 412,405
  • 93
  • 575
  • 722
0

Lets inspect the problem:
You want to return true when everything is true.

You can invert that requirement:
Return false when only one is false.

By that logic, you can can do the following:

for (int i = 0; i < instructionArea.size(); i++) {
        if(!instructionArea.get(i).getExecuted()) {
            return false; //when only one is false, it is not completed
        }
    }
return true; // we iterated over all elements and all were executed, hence it is completed
Raildex
  • 3,406
  • 1
  • 18
  • 42