i think it's at least marginally beneficial that the attributes are
"declared" somewhere so one doesn't need to read the whole class to take a
basic inventory of things.


On Mon, Oct 7, 2013 at 11:07 AM, Greg Price <notifications@github.com>wrote:

> In presenters/thread.rb:
>
> > @@ -0,0 +1,108 @@
> > +require 'new_relic/agent/method_tracer'
> > +
> > +class ThreadPresenter
> > +
> > +  def initialize(comment_threads, user, course_id)
> > +    @threads = comment_threads
> > +    @user = user
> > +    @course_id = course_id
> > +    @read_dates = nil # Hash, sparse, thread_key (str) => date
> > +    @unread_counts = nil # Hash, sparse, thread_key (str) => int
> > +    @endorsed_threads = nil # Hash, sparse, thread_key (str) => bool
>
> These initializations are unnecessary, since load_aggregates takes care of
> them. I think it would actually be reasonable to just inline
> load_aggregates, though it's fine if you disagree.
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/edx/cs_comments_service/pull/55/files#r6799537>
> .
>