0

My problem is, that I want to return an Object from the ArrayList "blocks". My code doesn't work - error says This method must return a result of type block

public block getBlockUnderneath (int x, int y){
    for(int i = 0; i<blocks.size(); i++){
         if (blocks.get(i).x == x) {
             return blocks.get(i);
         }
    }
}
4
  • What is blocks? Is it a List<block>? Commented Mar 2, 2014 at 18:19
  • 3
    What happens if blocks.size() returns 0? Commented Mar 2, 2014 at 18:19
  • 2
    What do you expect to happen if none of the blocks in the list matches x? (Also, I'd recommend that you learn about the enhanced for loop...) Commented Mar 2, 2014 at 18:20
  • for(Block b : blocks) { if() return b; } ... where List<Block> blocks; Commented Mar 2, 2014 at 18:22

3 Answers 3

2

You have two issues:

  1. If blocks.size()==0 your method returns nothing
  2. If none of the blocks in blocks have block.x==x your method returns nothing.

In Java a method must return a value of it is declared to do so.

The easiest solution to your issue is to return null at the end of the method:

public block getBlockUnderneath (int x, int y){
    for(final block b : blocks){
         if (b.x == x) {
             return b;
         }
    }
    return null;
}

Notice this uses an enhanced-for-loop, this is the recommended way to loop over Collections (or anything that implements Iterable<T>) in Java.

A better approach might be to throw an exception if no item is found:

public block getBlockUnderneath (int x, int y){
    for(final block b : blocks){
         if (b.x == x) {
             return b;
         }
    }
    throw new NoSuchElementException();
}

In either case you would need to handle the corner case in code that calls this method.

P.S. please stick to Java naming conventions. Classes should be in PascalCase - so you block class should be called Block.

Just for fun, in Java 8:

public block getBlockUnderneath(int x, int y) {
    return blocks.stream().filter((b) -> b.x == x).findFirst().get();
}
Sign up to request clarification or add additional context in comments.

3 Comments

Because that is the ways it was implement in current version due to time limit
@Vash I carried out a quick experiment (I was sure that you are wrong but I wanted to check) and you are wrong. The findFirst will only traverse the Stream until it finds the first matching element - filter will not traverse the entire Stream. Running (build 1.8.0-ea-b120.
You have right. My opinion wold be valid if you wold use forEach not filter and findFirst().
1

The problem with your method is that there exists a scenario in which the return block is not executed. In that case, when a method is not declared to be void, you must declare the exit point for it.

You can exit using return or throw an exception. The choice depends on what your program should do if the requested value could not be found.

public block getBlockUnderneath (int x, int y){
    for(int i = 0; i<blocks.size(); i++){
         if (blocks.get(i).x == x) {
             return blocks.get(i);
         }
    }
    return null; //or throw NoSuchElementException or IllegalStateException
}

What's more you can improve you code by using a for-each loop. This solution may give you better performance and also code security as it uses an iterator rather than accessing item by index which is not necessarily efficient.

In this case you access the same item twice.

if (blocks.get(i).x == x) {
       return blocks.get(i);
}

Full example

public Block findBlock(int x} { //The class name is Block

   for(Block block : blocks) {
      if(block.x == x {
         return block;  
      }
   }

    return null; 
}

Be also aware of that returning null may cause problems and thus is considered bad practice. You can avoid null, thanks to checked exceptions, default values or using Null object

There is a native implementation of this common coding pattern in Java 8. Using the Optional<T> class from the Guava library can solve this problem for versions of Java < 8.

public Optional<Block> findBlock(int x} { //The class name is Block

   for(Block block : blocks) {
      if(block.x == x {
         return Optional.of(block);  
      }
   }

    return Optional.empty(); 
}

Usage

  public void someActionWithBlocK() { 

      Optional<Block> block = findBlock(5);

      if(block.isPresent()) {
         //some action with block
      }
  }

Comments

0

You could never loop.

If you have a return statement inside of a loop, then the compiler doesn't take the bonafide guarantee that the loop will execute and that you will return. To get around that, you must also return after your loop.

Or, better yet, have one variable to return, like such:

block ret = null;
for(block b : blocks) {
    if(b.x == x) { // I'm going to go over this in a mo
        ret = b;
        break;
    }
}
return ret;

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.