On Wed, Mar 07, 2012 at 05:44:31PM +0000, Guido Trotter wrote:
> LGTM
>
> Thanks,
>
> Guido
>
> (maybe abstracting the generation of the error message would help
> making it more readable?)
Possibly. What do you think about:
diff --git a/htools/Ganeti/HTools/Cluster.hs b/htools/Ganeti/HTools/Cluster.hs
index 1f868f5..e9e423a 100644
--- a/htools/Ganeti/HTools/Cluster.hs
+++ b/htools/Ganeti/HTools/Cluster.hs
@@ -1003,8 +1003,9 @@ evacOneNodeInner :: Node.List -- ^ Cluster node
list
-> EvacInnerState -- ^ New best solution
evacOneNodeInner nl inst gdx op_fn accu ndx =
case applyMove nl inst (op_fn ndx) of
- OpFail fm -> either (const $ Left $ "Node " ++ Container.nameOf nl ndx ++
- " failed: " ++ show fm) (const accu) accu
+ OpFail fm -> let fail_msg = "Node " ++ Container.nameOf nl ndx ++
+ " failed: " ++ show fm
+ in either (const $ Left fail_msg) (const accu) accu
OpGood (nl', inst', _, _) ->
let nodes = Container.elems nl'
-- The fromJust below is ugly (it can fail nastily), but
I'm more worried if the construct "either (const …) (const x) x" is
readable, as opposed to:
case x of
Left _ -> …
Right _ -> x
thanks,
iustin
> On Tue, Mar 6, 2012 at 11:59 PM, Iustin Pop <[email protected]> wrote:
> > Instead of manually case-ing on the Either contents, let's just use
> > either with const functions (not really readable, but…).
> > ---
> > htools/Ganeti/HTools/Cluster.hs | 7 ++-----
> > 1 files changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/htools/Ganeti/HTools/Cluster.hs
> > b/htools/Ganeti/HTools/Cluster.hs
> > index 11b7cae..a8acd2c 100644
> > --- a/htools/Ganeti/HTools/Cluster.hs
> > +++ b/htools/Ganeti/HTools/Cluster.hs
> > @@ -999,11 +999,8 @@ evacOneNodeInner :: Node.List -- ^ Cluster
> > node list
> > -> EvacInnerState -- ^ New best solution
> > evacOneNodeInner nl inst gdx op_fn accu ndx =
> > case applyMove nl inst (op_fn ndx) of
> > - OpFail fm ->
> > - case accu of
> > - Right _ -> accu
> > - Left _ -> Left $ "Node " ++ Container.nameOf nl ndx ++
> > - " failed: " ++ show fm
> > + OpFail fm -> either (const $ Left $ "Node " ++ Container.nameOf nl ndx
> > ++
> > + " failed: " ++ show fm) (const accu) accu
> > OpGood (nl', inst', _, _) ->
> > let nodes = Container.elems nl'
> > -- The fromJust below is ugly (it can fail nastily), but
> > --
> > 1.7.9.1
