Working correct but requirements aren't

Pytania dotyczące zadania

Obliczanie wynagrodzeń

Podstawy Java, Poziom 9, Lekcja 11

Dyskutowane

Hey! Please take a look on my code. Should I use HashMap to get correct requirements or problem lies another place? 🤔🤯

Pierwszym parametrem metody main jest nazwa pliku.
Każdy wiersz określonego pliku ma następujący format:
name value
gdzie [name] to String, a [value] to double. [name] i [value] są oddzielone spacją.

Znajdź sumę wszystkich wartości dla każdego nazwiska.
Wyświetl wszystkie dane, posortowane według nazwisk w kolejności rosnącej.
Zamknij strumienie.

Przykładowy plik wejściowy:
Peterson 2 Smith 6 Baxter 1.35 Peterson 3.1

Przykładowy wynik:
Baxter 1.35 Peterson 5.1 Smith 6.0

Wymagania:

Program NIE może wczytywać danych z klawiatury.
Program musi odczytywać zawartość pliku (użyj konstruktora FileReader).
Strumień wejściowy pliku (FileReader) musi zostać zamknięty.
Program powinien zapisywać w konsoli każde nazwisko, posortowane w kolejności rosnącej oraz sumę wszystkich jego wartości.

package pl.codegym.task.task19.task1919; /* Obliczanie wynagrodzeń */ import java.io.*; import java.util.ArrayList; import java.util.Collections; import java.util.Iterator; public class Solution { public static void main(String[] args) throws Exception { File file = new File(args[0]);//"C:\\Users\\Daniel\\Desktop\\a.txt.txt" FileReader fr = new FileReader(file); BufferedReader reader = new BufferedReader(fr); ArrayList<String> mainList = new ArrayList<String>(); ArrayList<String> newList = new ArrayList<String>(); while(reader.ready()){ String txt = reader.readLine(); mainList.add(txt); } int position=1; Iterator it = mainList.iterator(); while(it.hasNext()){ String tekst = (String)it.next(); String name = getName(tekst); if(nameVsList(name, newList)){ continue; } Double money = getMoney(tekst); String addedName = checkAndAdd(name, money, mainList, position); position++; if(addedName!= null){ newList.add(addedName); } else { newList.add(tekst); } } Collections.sort(newList); for(String s : newList){ System.out.println(s); } fr.close(); reader.close(); } static String getName (String name) { int i = name.indexOf(" "); String name2 = name.substring(0,i); return name2; } static double getMoney (String money) { int i = money.indexOf(" "); Double money2 = Double.parseDouble(money.substring(i)); return money2; } static boolean nameVsList (String name, ArrayList<String> list){ boolean is= false; for(String s : list){ if(getName(s).equals(name)) { is = true; break; } } return is; } static String checkAndAdd(String name, double money, ArrayList<String>list, int pos){ // adds all the money for specified name, return null if not ocours String name3=null; double money3=0; for(int i = pos; i<list.size(); i ++){ String name1 = getName(list.get(i)); if(name.equals(name1)){ double money1 = getMoney(list.get(i)); money+= money1; money3+=money; name3 = name+ " " +money; } } return name3; } }

Komentarze (6)

Popularne
Najnowsze
Najstarsze

Musisz się zalogować, aby dodać komentarz

Thomas

Poziom 41 , Bayreuth, Germany

6 października 2022, 15:49

I am a bit puzzled why the validation does not recognize the FileReader. I'd try to initialize BufferedReader and FileReader in one line BufferedReader in = new BufferedReader(new FileReader(args[0])); and see what happens. If that doesn't help, then I'd use a try-with-resources block. Your code was quite hard to understand for me cause it is somewhat cumbersome. The methods partly with redundancies. The whole thing would be so much easier with a map. If not with a map, then you could at least program a class that stores name and value. In addition add getter and setter and implement the Comparable interface (compareTo method). The class would then even be sortable. Much better than using a string that has to be split at every access. Nevertheless the map approach is much simpler. You read a line from the file, extract name and value from it. If the name is not in the map, add name and value. If it is in the map, then add the value to the value in the map. With the getOrDefault method this part is a one-liner. If you use a TreeMap then it already is sorted and you do not need to take care of that anymore. If you want to use a HashMap, then get the keySet from the map, convert it to a list and sort that one. Now you can loop over that sorted list and grab the values from the map. Easy as that. I coded the approach using the POJO class. Just you see that it isn't really complex and pretty straight forward. It's pretty much your approach just bent to my style ;) But I really suggest to try the map way. It's easier and shorter and you probably will need the put element if not exists, add value if exists routine some more times in the future.

Thomas

Poziom 41 , Bayreuth, Germany

6 października 2022, 16:05

public static class DataSet implements Comparable<DataSet>, Serializable {
    private String name;
    private double value;
    private static final long serialVersionUID = 1L;

    public DataSet() {}

    public DataSet(String name, double value) {
        this.name = name;
        this.value = value;
    }

    public String getName() {
        return name;
    }

    public void setName(String name) {
        this.name = name;
    }

    public double getValue() {
        return value;
    }

    public void setValue(double value) {
        this.value = value;
    }

    public void addValue(double value) {
        this.value += value;
    }

    @Override
    public int compareTo(DataSet o) {
        return name.compareTo(o.name);
    }

    @Override
    public String toString() {
        return name + " " + value;
    }
}


private static void addDataSet(List<DataSet> list, DataSet element) {
    boolean exists = false;
    for(DataSet current : list) {
        if (current.getName().equals(element.getName())) {
            current.addValue(element.getValue());
            exists = true;
            break;
        }
    }
    if (!exists)
        list.add(element);
}

public static void main(String[] args) throws Exception {
    List<DataSet> data = new ArrayList<>();
    try (BufferedReader in = new BufferedReader(new FileReader("in.txt"))) {
        while (in.ready()) {
            String[] parts = in.readLine().split("\\s+");
            DataSet currSet = new DataSet(parts[0], Double.parseDouble(parts[1]));
            addDataSet(data, currSet);
        }
    } catch (IOException ioe) {
        throw new RuntimeException(ioe);
    }

    Collections.sort(data);
    data.forEach(System.out::println);
}

Dejejen

Poziom 22 , Poland, Poland

8 października 2022, 09:02

Thank You :) Even I chose easier way, and I did it using TreeMap. Still work great and still same result 🤪

public class Solution {
    public static void main(String[] args) throws Exception {
        File file = new File(args[0]);//"C:\\Users\\Daniel\\Desktop\\a.txt.txt");//);//
        FileReader fr = new FileReader(file);
        BufferedReader reader = new BufferedReader(fr);

        TreeMap<String, String> mainList = new TreeMap<>();

        double money = 0;

        while (reader.ready()) {
            String txt = reader.readLine();
            String[] tab = txt.split(" ");
            if (mainList.containsKey(tab[0])) {
                money = Double.parseDouble(mainList.get(tab[0])) + Double.parseDouble(tab[1]);
                mainList.put(tab[0], money + "");
            } else {
                mainList.put(tab[0], tab[1]);
            }
        }
        for (Map.Entry<String, String> m : mainList.entrySet()) {
            System.out.println(m.getKey() + " " + m.getValue());
        }
        fr.close();
        reader.close();
    }

}

Thomas

Poziom 41 , Bayreuth, Germany

8 października 2022, 10:00

Have you tried my initial suggestion to declare the reader in one line. If that does not work... try-with-resources. See below what I mean and also maps getOrDefault in action. Code isn't really tested but should work (I hope ;) )

public static void main(String[] args) throws Exception {
    TreeMap<String, Double> salaries = new TreeMap<>();

    try (BufferedReader in = new BufferedReader(new FileReader(args[0]))) {
        while (in.ready()) {
            String[] parts = in.readLine().trim().split("\\s");
            salaries.put(parts[0], salaries.getOrDefault(parts[0], 0.0) + Double.parseDouble(parts[1]));
        }
    }
    salaries.forEach((key, value) -> System.out.format("%s %s%n", key, value));
}

And what's the reason you opted for a String, String map? I'm just nosy... ;)

Dejejen

Poziom 22 , Poland, Poland

8 października 2022, 13:54

Thank You. Why String, String? Without reason 😅 You are right String,Double map is correct. That helped to get correct solution. Obviously I added try-catch.

Thomas

Poziom 41 , Bayreuth, Germany

8 października 2022, 16:53

ahh, OK, thanks :) Glad it worked out now and your map approach looks really good.