Confused why my code throws 'no return statement'

[Splanky222]

So, for my Data Structures class, we have to do a little Set Theory. The implementation asked for by the professor asks us to replace the toString() in the Object class with a toString that outputs the elements of the set (constrained to Integers from 0 to 10) as numbers separated by spaces, or "---" for an empty set. This is what I have, and I'm looking for a hint as to why I'm getting a 'missing return statement' error from the compiler. I have been trying to figure this out for a couple hours now, and it is the only error thrown. Counter is just an int counting the number of elements in the set, and set[] is an array of booleans; if set[n] == true, then n is an element of the set. i is initialized at the beginning of the IntegerSet class that this method is a part of. And yes, I know that there is a Set interface included with Java. This is an exercise for the class, so I feel like that would be (a) missing the point of the assignment, and (b) cheating.

Code: Select all

  public String toString() {
    if(counter == 0) //only true for an empty set
      return "---";
    else
      for(i = 0; i < 11; i++)
        if(set[i])
          return i + " ";
  }

Thanks XKCDers!

[phlip]

Java will refuse to compile a non-void function unless every possible code path ends in either a return or a throw statement. As far as the compiler can tell, it's possible for none of the "if" statements to activate (ie counter != 0 but set[i] is false for all i)", even if you've got the variables set up so that shouldn't happen.

Simplest way to fix it is to throw a RuntimeException at the bottom of the function - since if you get to that point, something's wrong with the values in your object.

As an aside - there's really no reason for i to be an instance variable... it should be a local in the function itself. Having it as an instance variable is only going to bring in the potential for confusion and bugs later on in the code (what happens if you have a loop elsewhere in the code that also happens to loop on i, and you call toString() in that loop?). Generally you only want to put stuff in instance variables if it is actually relevant to the state of the object as a whole.
On the other hand, the "11" value should be a class variable, specifically a "static final int SIZE = 11;". So that it's much easier to see when reading the code what the 11 represents, and it's also much easier to come in later and change the size of the set.

[Splanky222]

phlip,

Awesome! Thanks for the tip, especially the aside on style!

[jaap]

On the other hand, the "11" value should be a class variable, specifically a "static final int SIZE = 11;". So that it's much easier to see when reading the code what the 11 represents, and it's also much easier to come in later and change the size of the set.

I much prefer to use set.length instead of a constant variable.

[phlip]

Actually, yes, good point. That accomplishes the same thing (it's easy to tell, when looking at the loop, why it's looping that many times; it's easy to change the set's size without going through the whole class changing loop counters), but has the added bonuses that (a) it already exists in the array object, so you don't need to make your own, and (b) it opens it up even easier to making it so that separate Set objects can have different sizes, instead of one hard-coded size for all.

[Splanky222]

Two things:

First, the '11' only comes from the explicit assignment, which requested "sets of natural numbers from 0 to 10, using an array of booleans". That's what the client wanted!!! Theres no reason otherwise why it couldn't be opened up to an ArrayList<Double> that could take basically any real number, except for a little re-typing of code to get rid of the boolean related stuff.

Secondly, incidentally, there was something wrong with that code anyways, to where I actually didn't need the RuntimeException after all. The loop always returns after 1 iteration, so it only ever printed the first element in the set until I changed it to

Code: Select all

  public String toString() {
    String print = " ";
    if(counter == 0) //only true for an empty set
      return "---";
    else
        for(int i = 0; i < size; i++)
          if(set[i])
            print = print + i + " ";
    return print;
  }

[Yakk]

While the client only wanted things up to size 10, that is no reason for you to code something that doesn't work with nearly arbitrary sized things.

The nearly arbitrary sized version of the code will handle the size 10 case perfectly fine, and when requirements shift might also handle that.

[EivindEE]

Just thought I'd mention that the else clause is unnecessary. You could just write

Code: Select all

 public String toString() {
    String print = " ";
    if(counter == 0) //only true for an empty set
        return "---";
       
    for(int i = 0; i < set.length; i++){
        if(set[i]){
            print = print + i + " ";
        }
    }
    return print;
  }