How to remove duplication from my code

Question

I have two similar methods. One of them prints something and one of them save somethings. As you can see there are a lot of duplicate code. How should I refactor it and remove this duplication ?

public static void printSomething(List<String> list) {
    for (String item : list) {
        if (item.contains("aaa")) {
            System.out.println("aaa" + item);
        }
        if (item.contains("bbb")) {
            System.out.println("bbb" + item);
        } else {
            System.out.println(item);
        }
    }
}

public static Map<String, String> getSomething(List<String> list) {
    Map<String, String> map = new HashMap<String, String>();
    for (String item : list) {
        if (item.contains("aaa")) {
            map.put("aaa", item);
        }
        if (item.contains("bbb")) {
            map.put("bbb", item);
        } else {
            //do nothing
        }
    }
    return map;
}

UPDATE:

Code was updated to solve problem when method are not exactly similar

Answer 1

Assuming the order of which the println of "aaa" and "bbb" appear does not matter, you could replace the implementation of printSomething with

public static void printSomething(List<String> list) {
  Map<String, String> map = getSomething(list);
  for(Map.Entry<String, String> entry : map) {
    System.out.println(entry.getKey() + entry.getValue());
  }
}

Answer 2

A generic Interface Action that have a method action(T t) can reduce the code.

public interface Action<E> {
        void action(E e);
}

Example:

public static void forEach(List<String> list, Action <String> action) {
    for(String s : list){
           action.action(s);

}

Now you just need 2 different implementations of Action.

You can use annonymous types if you don't want to create a class.

If you know c# this is similar to lambdas.

edit:

Using annonymous type:

public static Map<String, String> getSomething(List<String> list) {
    final Map<String, String> map = new HashMap<String, String>();
    forEach(list, new Action<String>() {
        @Override
        public void action(String e) {
            if (e.contains("aaa")) {
                map.put("aaa", e);
            }
            if (e.contains("bbb")) {
                map.put("bbb", e);
            } else {
                // do nothing
            }
        }
    });
    return map;
}

Creating the class:

public static Map<String, String> getSomething2(List<String> list) {
    final Map<String, String> map = new HashMap<String, String>();
    forEach(list, new ListToMapAction(map));
    return map;
}


public class ListToMapAction implements Action<String> {

    Map<String, String> map;

    public ListToMapAction(Map<String, String> map) {
        this.map = map;
    }

    @Override
    public void action(String e) {
        if (e.contains("aaa")) {
            map.put("aaa", e);
        }
        if (e.contains("bbb")) {
            map.put("bbb", e);
        } else {
            // do nothing
        }
    }

}

Answer 3

In a programming language with first-class functions, you'd pass around a function as a parameter indicating what you want to do inside the loop (for an example see the update, below). Java is going to have lambdas in version 8, but they're not quite up to the job.

In the current state of Java, you'll have to settle with something uglier - for example, passing an extra parameter to the method; or you could pass around anonymous inner classes that implement an interface, but IMHO that's even uglier than what I'm about to suggest:

static void printSomething(List<String> list, boolean print)

If print is true then print inside the loop, otherwise add to the Map. Of course, you'll have to add a couple of ifs inside the loop for checking this condition, and at the beginning, one extra if to determine if the Map is to be initialized. Either way, the method returns a Map, but the Map can be null for the printing case. This is what I mean:

static Map<String, String> processSomething(List<String> list, boolean print) {

    Map<String, String> map = null;
    if (!print)
        map = new HashMap<String, String>();

    for (String item : list) {
        if (item.contains("aaa")) {
            if (print)
                System.out.println("aaa" + item);
            else
                map.put("aaa", item);
        }
        if (item.contains("bbb")) {
            if (print)
                System.out.println("bbb" + item);
            else
                map.put("bbb", item);
        } else if (print) {
            System.out.println(item);
        }
    }

    return map;

}

UPDATE

For example, in Python - which allows passing functions as parameters, this is how you'd solve the problem in an elegant fashion:

def processSomething(lst, func):
    result = None
    for item in lst:
        if 'aaa' in item:
            result = func(item, 'aaa', result)
        elif 'bbb' in item:
            result = func(item, 'bbb', result)
        else:
            result = func(item, '', result)
    return result

def printer(item, key, result):
    print key + item

def mapper(item, key, result):
    if not result:
        result = {}
    if key:
        result[key] = item
    return result

See how it works:

processSomething(['aaa', 'bbb', 'ccc'], printer)
=> aaaaaa
   bbbbbb
   ccc

processSomething(['aaa', 'bbb', 'ccc'], mapper)
=> {'aaa': 'aaa', 'bbb': 'bbb'}