I just meant that I think your changes solve more than half of the problem.


On Mon, Sep 23, 2013 at 11:52 AM, Luke Vilnis <notifications@github.com>wrote:

> In src/main/scala/cc/factorie/BP.scala:
>
> > @@ -28,41 +28,51 @@ trait BPRing {
> >    def newBPFactor(factor:Factor, varying:Set[DiscreteVar], summary:BPSummary): BPFactor
> >  }
> >
> > -object BPSumProductRing extends BPRing {
> > +// TODO this code duplication is why mixins are usually a bad way to factor stuff -luke
>
> I think it is useful to have TODO comments marking places that need
> further refactoring down the line, with an explanation of why. It's an
> acknowledgement to future maintainers that no, it's not just them, this
> code does look fishy. I suppose it could be rephrased?
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/factorie/factorie/pull/50/files#r6518381>
> .
>



-- 
 - Alexandre