Thanks for posting your code. You've done a good job with:
- formatting
- variable and method names
- relatively small, focused methods
You may be new to Ruby, but I don't think you're new to programming.
There's not much to be improved here. A few Ruby idioms you missed, and a few matters of style that aren't all that important.
Matters of Style
Line width
Some of the lines are a bit long. While there is no longer universal agreement that lines of code should be kept to less than 80 characters, there are many benefits to doing so. One of them is that, when you paste code into StackOverflow, the reader won't have to bother with horizontal scroll bars. Consider breaking long lines up. Usually, you can just hit enter where you want it, indent, and the code will still work fine. Sometimes, you'll need to add a \ (continuation) to the end of a line to let Ruby know that the rest of the statement is on the following line.
For example, this:
series_data << {name: "Cluster #{ index + 1 }", data: question_means_for_cluster}
could be formatted like this:
series_data << {
name: "Cluster #{ index + 1 }",
data: question_means_for_cluster
}
Vertical white space
There's more vertical white space than I'd use. I'd get rid of most, if not all of it. For example, instead of this:
clusters_for_axis = cluster(axis.id, i)
clusters_for_axis.each_with_index do |cluster_axis_summary, index|
cluster_name = cluster_axis_summary[:name]
this:
clusters_for_axis = cluster(axis.id, i)
clusters_for_axis.each_with_index do |cluster_axis_summary, index|
cluster_name = cluster_axis_summary[:name]
In general, I don't consider vertical white space within a method bad, in itself, but when it is used to separate lines into groups that have some similar purpose, I wonder if those lines could be moved into separate methods.
Code comments
Similarly, comments that say what the code is doing, while not bad, are better if they can be replaced with code that says what the comment used to say. Often the lines after a comment can be moved into a method with the method name serving the purpose the comment used to serve.
Ruby idioms
Let's let Ruby do some of the busywork for us.
Replacing a loop with reduce
In this code:
sum = 0
count = 0
# data_items is an array containing each respondent's survey scores (as an array)
cluster.data_items.each do |item|
sum += item[i]
count += 1
end
We are calculating a sum and a count. The count doesn't need to be calculated: just use cluster.data_items.size
. To calculate the sum, this code using reduce is idiomatic Ruby:
sum = cluster.data_items.reduce(:+)
Map
Ruby has a wonderful idiom for transforming one sequence to another, map. Whenever you see code like this:
series_data = []
clusterer.clusters.each_with_index do |cluster, index|
...
series_data << {
name: "Cluster #{ index + 1 }",
data: question_means_for_cluster
}
end
You can do this instead:
series_data = clusterer.clusters.map.with_index do |cluster, index|
series_data << {
name: "Cluster #{ index + 1 }",
data: question_means_for_cluster
}
end
Ruby frees you from the bother of creating the array and adding elements to it. map can also be used in #diana_clusterer.
self
There are a few uses of self
that you don't need. For example, this:
self.members.each do |member|
could be:
members.each do |member|
See this SO question for more.