0

Here's my code that returns a mark based on average score of tests done by a person.

    public Map<Person, String> defineMarks(Stream<CourseResult> results) {
    return results.collect(Collectors.toMap(CourseResult::getPerson,x->{
        double avg=x.getTaskResults().values().stream().collect(Collectors.summarizingInt(Integer::intValue)).getAverage();
        if(avg > 90) return "A";
        if(avg >= 83) return "B";
        if(avg >= 75) return "C";
        if(avg >= 68) return "D";
        if(avg >= 60) return "E";
        else return "F";
    }));
}

For reference here's CourseResult class

public class CourseResult {
    private final Person person;
    private final Map<String, Integer> taskResults;

    public CourseResult(final Person person, final Map<String, Integer> taskResults) {
        this.person = person;
        this.taskResults = taskResults;
    }

    public Person getPerson() {
        return person;
    }

    public Map<String, Integer> getTaskResults() {
        return taskResults;
    }
}

and 2 methods that generates test scores. historyResults

private final String[] practicalHistoryTasks = {"Shieldwalling", "Phalanxing", "Wedging", "Tercioing"};

private Stream<CourseResult> historyResults(final Random random) {
    int n = random.nextInt(names.length);
    int l = random.nextInt(lastNames.length);
    AtomicInteger t = new AtomicInteger(practicalHistoryTasks.length);

    return IntStream.iterate(0, i -> i + 1)
            .limit(3)
            .mapToObj(i -> new Person(
                    names[(n + i) % names.length],
                    lastNames[(l + i) % lastNames.length],
                    18 + random.nextInt(20)))
            .map(p -> new CourseResult(p,
                    IntStream.iterate(t.getAndIncrement(), i -> t.getAndIncrement())
                            .map(i -> i % practicalHistoryTasks.length)
                            .mapToObj(i -> practicalHistoryTasks[i])
                            .limit(3)
                            .collect(toMap(
                                    task -> task,
                                    task -> random.nextInt(51) + 50))));
}

and programmingResults

    private final String[] programTasks = {"Lab 1. Figures", "Lab 2. War and Peace", "Lab 3. File Tree"};

 private Stream<CourseResult> programmingResults(final Random random) {
    int n = random.nextInt(names.length);
    int l = random.nextInt(lastNames.length);

    return IntStream.iterate(0, i -> i + 1)
            .limit(3)
            .mapToObj(i -> new Person(
                    names[(n + i) % names.length],
                    lastNames[(l + i) % lastNames.length],
                    18 + random.nextInt(20)))
            .map(p -> new CourseResult(p, Arrays.stream(programTasks).collect(toMap(
                    task -> task,
                    task -> random.nextInt(51) + 50))));
}

So there are 3 test scores for each person. If I generate scores using programmingResults its fine because there are only 3 programming tests but if I generate scores using historyResults I also get 3 test scores per person but I should treat it as if the 4th test was not attempted at all which means 0 points for it. How can I make my defineMarks method get an average of collected test scores and in the case of historyResults get an average of collected test scores and a score of 0.

If my question is unclear please instruct me how to make it better :D

fireballun
  • 61
  • 5
  • Just to clarify, you don't want to consider the fourth value not because it's 0, but because it has not been scored yet and 0 is just your way of knowing it, right? Like, you want to filter out tests that do not have been scored yet. So if only 3 tests were scored, the average will be /3 instead of /4. Is that correct? – wleao Jul 02 '22 at 13:32
  • Then I would suggest you starting using a filter (quick reference: https://mkyong.com/java8/java-8-filter-a-map-examples/) in your stream to remove the entry values that are 0 in order for you to get a sense of what you want. After that, I think the best approach would be to have a flag that will acknowledege which tests sould be scored or not. Or, you could leave the task result out of the List. Like, only tasks that have actually been scored should be in the collection. If that makes any sense, let me know and I will add it as an answer. – wleao Jul 02 '22 at 13:35
  • @wleao Sorry, I misunderstood earlier. No, the average should be /4 but there are 3 tests tied to a person so my method gets an average out of tests that have been scored/ are in a CourseResult object. – fireballun Jul 02 '22 at 13:36
  • Oh.. okay, then I think you should not be using getAverage. If you know the amount of tests, then add all the current results (call sum) and divide it by a specified N value that should be provided by your CourseResult or other object. By adding everything, if a test is not there, it is virtually considered to be zero. – wleao Jul 02 '22 at 13:38
  • @wleao Hey, sorry but I forgot to include that there are 2 score generating methods. Can I detect in stream which one was used or something xd? – fireballun Jul 02 '22 at 13:46
  • Do you want to get the average of everything or two separate averages? Will you have two different CourseResult objects per person? – wleao Jul 02 '22 at 13:50
  • @wleao I will have only one CourseResult per person. Some course results will be generated using historyResults and some using programminResults. – fireballun Jul 02 '22 at 13:56
  • It sounds like you want to [add a new value to an existing stream](https://stackoverflow.com/q/28785833/12567365), which is not how streams work. You can try [`concat()`](https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/util/stream/Stream.html#concat(java.util.stream.Stream,java.util.stream.Stream)). – andrewJames Jul 02 '22 at 13:59
  • Alternatively (unless this is a "only use streams" exercise), you can rethink the approach - return a `List` instead of a `Stream` from `programmingResults()`, and adjust your logic accordingly. Also, handling a plain list should make the code clearer & easier to maintain, I think. You can do that and still use streams - judiciously. – andrewJames Jul 02 '22 at 13:59
  • Why do you think it makes sense to reduce the score of a student which hasn't yet attempted to pass of the tests (if they failed it completely) ? I think you don't want to experience something similar in a real life situation. – Alexander Ivanchenko Jul 02 '22 at 14:00
  • @AlexanderIvanchenko chill, it's just an exercise. And it's not "hasn't yet attempted". It's "hasn't attempted". And I experienced something similar friend. I got too late to an exam. – fireballun Jul 02 '22 at 14:10
  • @andrewJames it's not streams only but unfortunately I can only modify defineMarks method and maybe add a new one. Since the 4th unattempted test score is 0 its more like using a custom average method I guess cuz I don't have to add a 0. – fireballun Jul 02 '22 at 14:11
  • "_I can only modify defineMarks ..._" - Understood - but you should probably make that (and any other constraints) clear in the body of the question. Points like that can get lost in a long string of comments. – andrewJames Jul 02 '22 at 14:22
  • @fireballun `it's not "hasn't yet attempted". It's "hasn't attempted"` - well, that's indeed different. If I were you, I would treat this case by creating a `CourseResult` with a score `0`. Event happens - data comes. There was a test - there should be an explicit result. That would be cleaner. – Alexander Ivanchenko Jul 02 '22 at 14:24
  • @andrewJames ah yes, I really should do that! – fireballun Jul 02 '22 at 14:28

1 Answers1

1

I don't quite understand the point of the exercise. But if you want to add only one value to your stream and you can't modify the test score producing methods (there it would be easy to change the limit(3) to limit(4)) then may be you can use the task names to change the way of your calculation of the average, since all programTasks start with Lab ...:

return results.collect(
        Collectors.toMap(
                CourseResult::getPerson,
                x -> {
                    boolean areProgramTasks = x.getTaskResults().keySet().stream().allMatch(key -> key.startsWith("Lab "));
                    double avg;
                    if(areProgramTasks){
                        avg = x.getTaskResults().values().stream().collect(Collectors.summarizingInt(Integer::intValue)).getAverage();
                    }
                    else {
                        avg = Stream.concat(Stream.of(0), x.getTaskResults().values().stream()).collect(Collectors.summarizingInt(Integer::intValue)).getAverage();
                    }
                    if(avg >  90) return "A";
                    if(avg >= 83) return "B";
                    if(avg >= 75) return "C";
                    if(avg >= 68) return "D";
                    if(avg >= 60) return "E";
                    else return "F";
                }
        ));
Eritrean
  • 15,851
  • 3
  • 22
  • 28
  • Well, to be honest I also don't get why they complicated it needlessly with that limit in historyResults. Anyways, thank you very much. I even almost fully understand the code aside from summarizingInt that I don't quite get how it works but will read some documentation on it. – fireballun Jul 02 '22 at 16:23
  • @fireballun It is just a collector which returns an [IntSummaryStatistics](https://docs.oracle.com/javase/8/docs/api/java/util/IntSummaryStatistics.html) which are very handy if you want to get the `max` , `min`, `avg`, `count` , `sum` of a dataset in a single pass – Eritrean Jul 02 '22 at 16:33