Помогите найти ошибку

Вопрос по задаче

Архивный

public class Solution {
    public static HashMap<String, String> createMap() {
        HashMap<String, String> map = new HashMap<String, String>();
        map.put("Тупов","Эдуард");
        map.put("Тупов2","Дима");
        map.put("Тупов3","Артем");
        map.put("Ющенко","Эдуард");
        map.put("Ющенко2","Дима");
        map.put("Ющенко3","Артем");
        map.put("Смежный","Эдуард");
        map.put("Смежный2","Дима");
        map.put("Смежный3","Артем");
        map.put("Эдуард1","Эдуард1");

        return map;//напишите тут ваш код
    }

    public static void removeTheFirstNameDuplicates(HashMap<String, String> map) {
        HashMap<String, String> copy = new HashMap<String, String>(map);
        for (String s : map.values()) {
            if ((Collections.frequency(copy.values(), s)) > 1)
                removeItemFromMapByValue(map, s);//напишите тут ваш код
        }
    }

    public static void removeItemFromMapByValue(HashMap<String, String> map, String value) {
        HashMap<String, String> copy = new HashMap<String, String>(map);
        for (Map.Entry<String, String> pair : copy.entrySet()) {
            if (pair.getValue().equals(value))
                map.remove(pair.getKey());
        }
    }

    public static void main(String[] args) {
    }
}

Создать словарь (Map<String, String>) занести в него десять записей по принципу "фамилия" - "имя".
Удалить людей, имеющих одинаковые имена.

Требования:

Программа не должна выводить текст на экран.
Программа не должна считывать значения с клавиатуры.
Метод createMap() должен создавать и возвращать словарь Map с типом элементов String, String состоящих из 10 записей.
Метод removeTheFirstNameDuplicates() должен удалять из словаря всех людей, имеющие одинаковые имена.
Метод removeTheFirstNameDuplicates() должен вызывать метод removeItemFromMapByValue().

package com.javarush.task.task08.task0817; import java.util.Collections; import java.util.HashMap; import java.util.Map; /* Нам повторы не нужны */ public class Solution { public static HashMap<String, String> createMap() { HashMap<String, String> map = new HashMap<String, String>(); for (int i = 0; i < 10; i++) { map.put("surName" + i, "name" + i); } return map;//напишите тут ваш код } public static void removeTheFirstNameDuplicates(HashMap<String, String> map) { HashMap<String, String> copy = new HashMap<String, String>(map); for (String s : map.values()) { if ((Collections.frequency(copy.values(), s)) > 1) removeItemFromMapByValue(map, s);//напишите тут ваш код } } public static void removeItemFromMapByValue(HashMap<String, String> map, String value) { HashMap<String, String> copy = new HashMap<String, String>(map); for (Map.Entry<String, String> pair : copy.entrySet()) { if (pair.getValue().equals(value)) map.remove(pair.getValue()); } } public static void main(String[] args) { } }

Комментарии (8)

популярные
новые
старые

Для того, чтобы оставить комментарий Вы должны авторизоваться

Максим Фёдоров

Уровень 20

23 января 2018, 10:35

Разобрался, переписал метод removeTheFirstNameDuplicates

public static void removeTheFirstNameDuplicates(HashMap<String, String> map) {
    HashMap<String, String> copy = new HashMap<String, String>(map);
    for (Map.Entry<String, String> pair : copy.entrySet()) {
        if ((Collections.frequency(copy.values(), pair.getValue())) > 1)
            removeItemFromMapByValue(map, pair.getValue());//напишите тут ваш код
    }
}

Антон Никольский

Уровень 35

Expert

23 января 2018, 10:12

Внутри цикла for-each нельзя удалять элементы (такое удаление нарушает работу внутреннего итератора, который осуществляет перебор элементов). Нужно сделать цикл с помощью итератора и удалять методом remove итератора, чтобы он об этом знал.

Максим Фёдоров

Уровень 20

23 января 2018, 10:58

Так ?

public static void removeTheFirstNameDuplicates(HashMap<String, String> map) {
    HashMap<String, String> copy = new HashMap<String, String>(map);
    for (Iterator<Map.Entry<String, String>> iterator = copy.entrySet().iterator(); iterator.hasNext();) {
        Map.Entry<String, String> pair = iterator.next();
        if ((Collections.frequency(copy.values(), pair.getValue())) > 1)
            removeItemFromMapByValue(map, pair.getValue());
    }
}

Антон Никольский

Уровень 35

Expert

23 января 2018, 11:25

Мой комментарий касался метода removeItemFromMapByValue, и он был неверный, т.к. я по невнимательности не заметил строки с копированием мапы. Идти циклом по одной мапе, удаляя при этом из другой, вполне можно. Другое дело, что операция копирования довольно затратная. Весь смысл явного применения итератора - именно в удалении методом remove итератора, если не было копирования всей мапы или коллекции. Поэтому в вашем случае смысла в его применении нет.

Максим Фёдоров

Уровень 20

23 января 2018, 11:46

Понял, я также не внимательно посмотрел. Все верно, правильно было бы в методе removeItemFromMapByValue удалять через итератор, а не так как было представлено изначально, но в данном случае такой вариант проходит проверку и работает:

public static void removeTheFirstNameDuplicates(HashMap<String, String> map) {
    HashMap<String, String> copy = new HashMap<String, String>(map);
    for (Map.Entry<String, String> pair : copy.entrySet()) {
        if ((Collections.frequency(copy.values(), pair.getValue())) > 1)
            removeItemFromMapByValue(map, pair.getValue());//напишите тут ваш код
    }
}

Максим Фёдоров

Уровень 20

23 января 2018, 11:51

А метод removeItemFromMapByValue лучше реализовать так:

public static void removeItemFromMapByValue(HashMap<String, String> map, String value) {
    HashMap<String, String> copy = new HashMap<String, String>(map);
    for (Iterator<Map.Entry<String, String>> iterator = copy.entrySet().iterator(); iterator.hasNext();) {
        Map.Entry<String, String> pair = iterator.next();
        if (pair.getValue().equals(value))
            map.remove(pair.getKey());
    }
}

Антон Никольский

Уровень 35

Expert

23 января 2018, 14:12

Да, в последнем removeTheFirstNameDuplicates также копирование мапы, поэтому все работает. В последнем removeItemFromMapByValue также копирование мапы, поэтому явный итератор не нужен. Если без копирования, я бы сделал его так:

public static void removeItemFromMapByValue(HashMap<String, String> map, String value) {
    for (Iterator<Map.Entry<String, String>> iterator = map.entrySet().iterator(); iterator.hasNext();) {
        Map.Entry<String, String> pair = iterator.next();
        if (pair.getValue().equals(value))
            iterator.remove();
    }
}

В этом случае копирования нет и, соответственно, удаление производится через iterator.remove(). Но если уж искать идеальный (по скорости и затрате ресурсов) алгоритм, то я бы вообще сделал по-другому. Сначала прошелся бы по мапе, находя все дублирующие значения и скидывая их в HashSet. Затем второй раз прошелся, удаляя эти значения из мапы. Поиск в HashSet очень быстрый (по хешу, без перебора). Итого всего два прохода по мапе и без копирования всей мапы. В данной задаче мапа полностью перебирается при каждом удалении, плюс копируется каждый раз.

Максим Фёдоров

Уровень 20

23 января 2018, 14:43

спасибо, буду иметь ввиду