code duplication, with similar code in methods doing the opposite thing with multiple objects

Question

I was wondering if there was a way to remove this sort of duplicate code (it crops up all across my program)

public void pickUp(Cointainer in)
{
    if(playerContainer.returnSomething() == null)
    {
        playerContainer.setSomething(in.returnSomething());
        in.setSomething(null);
    }
}

public void dropContainer (Container out)
{
    if(out.returnSomething() == null)
    {
        out.setSomething(playerContainer.returnSomething());
        playerContainer.setSomething(null);
    }
}

As you can see from the example above, the two methods are essentially exactly the same, except for which container is tested to be null, and which container the object ends up in. Is there anyway to reduce something like this to one method? Another example occurs later in the code, where multiple objects are tested in a different order:

if (control = 1)
{
  if(con1 == null)
    return con1
  else if (con2 == null)
    return con2
  else
    return con3
}
else
{
  if(con3 != null)
    return con3
  if(con2 != null)
    return con2
  else
    return con1
}

Is there anyway for this type of statement to be reduced to one if?

Sorry if my question is really noobish/retarded, I might be a bit lacking on that side of things, specially considering the last question I asked here :/

Anyway, thanks for taking the time to read this :)

Answer 1

You can introduce a new method which actually does the job of moving from one to other and express the other two in terms of the common functionality.
pickup = move from in -> player
drop = move from player -> out

public void pickUp(Container in)
{
    moveContainer(in, playerContainer);
}

public void dropContainer (Container out)
{
   moveContainer(playerContainer, out);
}

public void moveContainer(Container from, Container to) {
    if (to.returnSomething() == null) {
        to.setSomething(from.returnSomething());
        from.setSomething(null);
    }
}

Answer 2

First things first, you should clearly define what each function does. The underlying reason your code is duplicated is probably a developer needs that functionality and then implements it without knowing the exact functionality is already implemented. To prevent this happening developers should know where to look for that functionality.

I highly recommend you to read Single Responsibility Principle. Without using object oriented principles, the problem will most probably occur again.

For the second case, I would recommend merging the conditions if possible. But this would heavily depend on what your conditions are.

Answer 3

I'll deal with that making, if possible, every class I want to use in this way implement the same interface. For example, I would create the PlayerContainer class implement a new Doable interface containing the returnSomething and setSomething methods. Then, the method would take two arguments of type Doable, one per each object to work width, resulting in:

public interface Doable {
    Object returnSomething();
    setSomething(Object);
}

class MyClass {
    public void pickUp(Doable one, Doable two) {
        if(two.returnSomething() == null) {
            two.setSomething(one.returnSomething());
            one.setSomething(null);
        }
    }
}

EDIT: Note that the return type of returnSomething and the parameter of setSomething should be adjusted to one they actually use.

And, of course, this can be applied to most parts of code.

If you don't have the code of the classes you need to make implement the interface you can extend them and make the subclass to implement the interface.

Answer 4

For the second part as Dev Blanked has already suggested you can do something like this. But this adds extra space and time complexity.

private Object getMatch(Object obj1, Object obj2, Object obj3, boolean reverse) {
    List<Object> availableObjects = new ArrayList<Object>();
    availableObjects.add(obj1);
    availableObjects.add(obj2);
    availableObjects.add(obj3);
    int size = availableObjects.size();
    for (int i = 0 ; i < size ; i++) {
        Object nxtObject = reverse ? availableObjects.get(size - i -1) :  availableObjects.get(i);
        if (nxtObject != null) {
            return nxtObject;
        }
    }
    return null;
}