Dear Klaus,

Thanks for the review. I agree with you and I'll resend the patch soon.

Sincerely,
Oleg

On Aug 24, 2015 12:38 PM, "Klaus Aehlig" <[email protected]> wrote:
>
> On Fri, Aug 21, 2015 at 03:31:14AM +0300, Oleg Ponomarev wrote:
> > Exclude components which are equal to zero from the computation sum
> > on compile time by using Maybe monad to wrap optimal value computation
> > function.
> >
> > Signed-off-by: Oleg Ponomarev <[email protected]>
> > ---
> >  src/Ganeti/HTools/Cluster/MetricsComponents.hs | 17 ++++-------------
> >  src/Ganeti/HTools/Cluster/MetricsTH.hs         | 17 ++++++++++++-----
> >  2 files changed, 16 insertions(+), 18 deletions(-)
> > -  let comp c = appE (optimalValue c) $ varE nl
> > -      stat = appE [| sum :: [Double] -> Double |] . listE $ map comp
components
> > +  let stat = foldl (addVal nl) [| 0 :: Double |] $ map optimalValue
components
> >        fname = mkName "optimalCVScore"
> >    sig_d <- sigD fname ((arrowT `appT` [t| Node.List |]) `appT` [t|
Double |])
> >    fun_d <- funD fname [clause [varP nl] (normalB stat) []]
> >    return [sig_d, fun_d]
> > +  where
> > +    addVal :: Name -> ExpQ -> Maybe ExpQ -> ExpQ
> > +    addVal nl cur (Just f) = appTwice [| (+) :: Double -> Double ->
Double |]
> > +                                      cur . appE f $ varE nl
> > +    addVal _ cur Nothing = cur
>
> The addVal basically filters out the the Just values; I think the code
> would be slightly more readable if you have
>
>   +  let stat = foldl (addVal nl) [| 0 :: Double |] $ mapMaybe
optimalValue components
>
> and
>
>   +   addVal :: Name -> ExpQ -> ExpQ -> ExpQ
>
> What do you think?
>
> Otherwise, looks good. Thanks!
>
> --
> Klaus Aehlig
> Google Germany GmbH, Dienerstr. 12, 80331 Muenchen
> Registergericht und -nummer: Hamburg, HRB 86891
> Sitz der Gesellschaft: Hamburg
> Geschaeftsfuehrer: Graham Law, Christine Elizabeth Flores

Reply via email to