Krdytkyu

I have a map for which the values are a collection. Given a key, I want to remove an element of the collection and return it, but I also want to remove the entry if the collection is empty. Is there a way to do this in a short way using one of the numerous new Map methods of Java 8?

One easy example (I use a Stack but it could be a List, a Set, etc.). For the sake of the example, let's say that it is already checked that the map contains the key.

public static String removeOne(Map<Integer, Stack<String>> map, int key) {

    Stack<String> stack = map.get(key);

    String result = stack.pop();

    if(stack.isEmpty()){

        map.remove(key);

    }

    return result;

}

I tried doing something like

map.compute(1, (k, v) -> {v.pop(); return v.size() == 0 ? null : v;});

But even though it does indeed remove the entry if empty, I don't know how to get the value returned by pop().

Is there a way to do this in a short way using one of the numerous new
Map methods of Java 8?

There's no new method as of JDK8 that would improve your code whether that's in terms of readability or efficient.

if you're doing this as an exercise for your self then I can understand to some extent why you'd want to shorten the code (if possible) but when it comes to production code golfing should be avoided and instead go with the approach that is most readable and maintainable; doesn't matter if it's longer.

Your approach is good as is.

Or you could possibly rewrite it using size as:

public static String removeOne(Map<Integer, Stack<String>> map, int key) {

    return map.get(key).size() == 1 ? map.remove(key).pop() : map.get(key).pop();

}

Well, it's even uglier than what you have already in place, but there is a way, I guess:

public static String removeOne(Map<Integer, Stack<String>> map, int key) {

    String removed = new String[1];

    map.compute(key, (k, v) -> {

        removed[0] = v.pop();

        return v.size() == 0 ? null : v;

    });

    return removed[0];

}

Problem is that merge/compute and the like return the value, and in your case that is a Stack/Set/List, not and individual element from that collection.

Guava's Multimap handles the remove-collection-if-empty logic for you. You can get equivalent behaviour to your method in two lines:

public static String removeOne(ListMultimap<Integer, String> map, int key) {

    List<String> stack = map.get(key);

    return stack.remove(stack.size() - 1);

}

Both your existing solution and the above throw Exceptions if the map has no entries for the given key. Optionally you can change the code to handle this:

public static String removeOne(ListMultimap<Integer, String> map, int key) {

    List<String> stack = map.get(key);

    if (stack.isEmpty()) {

        return null;

    }

    return stack.remove(stack.size() - 1);

}

And of course you can make this generic:

public static <K, V> V removeOne(ListMultimap<K, V> map, K key) {

    List<V> stack = map.get(key);

    if (stack.isEmpty()) {

        return null;

    }

    return stack.remove(stack.size() - 1);

}

I fully agree with @NicholasK. There's no reason to use any streams or lambdas here.

Your approach is pretty good. The only thing I would like to add is making it generic:

public static <K, E, C extends Collection<E>> E removeOne(Map<K, C> map, K key) {

    C col = map.get(key);

    Iterator<E> it = col.iterator();

    E e = it.next();

    it.remove();

    if (!it.hasNext()) {

        map.remove(key);

    }

    return e;

}

This method will be applicable to any collections (map values) returning valid iterator.

/* quite ugly

String rv = Optional.ofNullable(map.get(1)).map(stack -> {

            if (!stack.isEmpty()) {

                String v = stack.pop();

                if (stack.isEmpty()) {

                    map.remove(1);

                }

                return v;

            }

            return null;

        }).orElse(null);

*/ 



@Test

public void test() {

    {

        Map<Integer, Stack<String>> map = new HashMap<>();

        Stack<String> s = new Stack<String>();

        s.addAll(Arrays.asList("a", "b"));

        map.put(1, s);

        String rv = Optional.ofNullable(map.get(1)).map(stack -> {

            if (!stack.isEmpty()) {

                String v = stack.pop();

                if (stack.isEmpty()) {

                    map.remove(1);

                }

                return v;

            }

            return null;

        }).orElse(null);

        Assert.assertEquals("b", rv);

        Assert.assertEquals(1, map.get(1).size());

        Assert.assertEquals("a", map.get(1).iterator().next());

    }

    {

        Map<Integer, Stack<String>> map = new HashMap<>();

        Stack<String> s = new Stack<String>();

        s.add("a");

        map.put(1, s);

        String rv = Optional.ofNullable(map.get(1)).map(stack -> {

            if (!stack.isEmpty()) {

                String v = stack.pop();

                if (stack.isEmpty()) {

                    map.remove(1);

                }

                return v;

            }

            return null;

        }).orElse(null);

        Assert.assertEquals("a", rv);

        Assert.assertNull(map.get(1));

    }

}