On Wed, Sep 28, 2011 at 6:05 PM, Iustin Pop <[email protected]> wrote:

> On Tue, Sep 27, 2011 at 05:16:16PM +0200, Agata Murawska wrote:
> > On Tue, Sep 27, 2011 at 5:12 PM, Iustin Pop <[email protected]> wrote:
> > > On Tue, Sep 27, 2011 at 05:09:14PM +0200, Agata Murawska wrote:
> > >> On Mon, Sep 26, 2011 at 12:08 PM, Iustin Pop <[email protected]>
> wrote:
> > >> > +-- | Sums two 'AllocSolution' structures.
> > >> > +sumAllocs :: AllocSolution -> AllocSolution -> AllocSolution
> > >> > +sumAllocs (AllocSolution asf asa asol asl)
> > >> > +          (AllocSolution bsf bsa bsol bsl) =
> > >> > +    -- note: we add b first, since usually it will be smaller; when
> > >> > +    -- fold'ing, a will grow and grow whereas b is the per-group
> > >> > +    -- result, hence smaller
> > >> > +    let nsf = bsf ++ asf
> > >> > +        nsa = asa + bsa
> > >> > +        nsols = compareAE asol bsol
> > >> > +        nsl = bsl ++ asl
> > >> > +    in AllocSolution nsf nsa nsols nsl
> > >> > +
> > >> I would consider using a little longer names for the sake of
> > >> readability (or at least capitalize the last letter). Also, the as_
> > >> make sense when one looks at the AllocSolution definition, but bs_ do
> > >> not.
> > >
> > > Ah. 'a' and 'b' were intended to be simply counters, as they seemed to
> > > me better than '1' and '2'.
> > They are far more haskell-ish, for sure (at least from my
> > it-was-a-long-long-long-time-ago experience)
> >
> > >
> > >> Maybe aFl aAl aSol aLog?
> > >
> > > Sounds good, but that still leaves the a/b issue. Should I change it
> > > back to aFl1 aAl1 aSol1 aLog1, respective aFl2 aAl2 aSol2 aLog2?
> > Either that or just aFl bFl - in both cases the distinction is clear
> > and I think it is a little easier to decipher what they stand for
>
> Interdiff:
>
>
> diff --git a/htools/Ganeti/HTools/Cluster.hs
> b/htools/Ganeti/HTools/Cluster.hs
> index aaeeb06..5f4a8d1 100644
> --- a/htools/Ganeti/HTools/Cluster.hs
> +++ b/htools/Ganeti/HTools/Cluster.hs
> @@ -630,16 +630,16 @@ concatAllocs as (OpGood ns) =
>
>  -- | Sums two 'AllocSolution' structures.
>  sumAllocs :: AllocSolution -> AllocSolution -> AllocSolution
> -sumAllocs (AllocSolution asf asa asol asl)
> -          (AllocSolution bsf bsa bsol bsl) =
> +sumAllocs (AllocSolution aFails aAllocs aSols aLog)
> +          (AllocSolution bFails bAllocs bSols bLog) =
>      -- note: we add b first, since usually it will be smaller; when
>     -- fold'ing, a will grow and grow whereas b is the per-group
>     -- result, hence smaller
> -    let nsf = bsf ++ asf
> -        nsa = asa + bsa
> -        nsols = compareAE asol bsol
> -        nsl = bsl ++ asl
> -    in AllocSolution nsf nsa nsols nsl
> +    let nFails  = bFails ++ aFails
> +        nAllocs = aAllocs + bAllocs
> +        nSols   = bestAllocElement aSols bSols
> +        nLog    = bLog ++ aLog
> +    in AllocSolution nFails nAllocs nSols nLog
>
>  -- | Given a solution, generates a reasonable description for it.
>  describeSolution :: AllocSolution -> String
>
> --
> iustin
>
LGTM

Reply via email to