Basic statistics implementation, not yet integrated.

[stopping]

Implementation of statistics objects, including methods for scoring individual courses and aggregating data for multiple courses (e.g.
ranking).

[gunsch]

got a better name than ConcreteAggregator?

[stopping]

My personal convention recently has been to name classes 'Standard[interface name]" for interfaces which only have one real implementation (not including test classes). Any better alternatives?

[gunsch]

For [interface] and single-implementation, I prefer the "[Interface]Impl" convention. But ideally I'd prefer a descriptive name, particularly since it's an "aggregator" and should describe what kind of aggregation it is doing. RankingDataAggregator?

Note that if you don't have a reason to split the interface/implementation out, you can also just combine them into one class. Guice isn't restricted to injecting interfaces, so injecting a concrete class could make sense if you don't expect to have other aggregators going forward.

There are definitely a few cases in the codebase right now of an interface with a single implementation, but most of those have an intention (or at least a plausible possibility) to have multiple future implementations.

[stopping]

There are definitely a few cases in the codebase right now of an interface with a single implementation, but most of those have an intention (or at least a plausible possibility) to have multiple future implementations.

Also unit tests. :)

[stopping]

Maybe we can change DataAggregator so that it just implements a single "aggregate" method. This way we can have sorting aggregators, grouping aggregators, etc. Maybe genericize DataAggregator with the return type of the aggregate method?

public class RankingDataAggregator implements DataAggregator<List>

[gunsch]

That seems reasonable, what would the return type be though? Why not always
a Rateable?

On Tue, May 5, 2015 at 12:09 PM, Sean Topping [email protected]
wrote:

In src/main/java/com/whichclasses/statistics/ConcreteAggregator.java
#3 (comment):

+package com.whichclasses.statistics;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+
+import com.google.common.collect.Maps;
+import com.google.inject.Inject;
+import com.whichclasses.model.Course;
+import com.whichclasses.model.DataSource;
+import com.whichclasses.model.Department;
+import com.whichclasses.model.proto.TceRating.Question;
+
+public class ConcreteAggregator implements DataAggregator {

Maybe we can change DataAggregator so that it just implements a single
"aggregate" method. This way we can have sorting aggregators, grouping
aggregators, etc. Maybe genericize DataAggregator with the return type of
the aggregate method?

public class RankingDataAggregator implements DataAggregator>

—
Reply to this email directly or view it on GitHub
https://github.com/cleichner/whichclasses/pull/3/files#r29702022.

[stopping]

I guess the intent of the class is to answer queries such as:
"Give me a list of rated courses in CSC."
"Give me a list of rated professors in AME."
"Give me a list of rated semesters for ACCT." (how are the classes in ACCT changing over time?)

Come to think of it, maybe the aggregator should always just return a list of Rateable objects. Then you can do sorting and selection as needed. Basically it's just a way to convert a large collection of individual class ratings into chunked rated collections (e.g. courses, departments, instructors, semesters, etc). Basically a GROUP BY implementation.