Refactor. #82

[epost]

@marcosh @wisnesky Ok this is a largely random batch of refactorings, and I have some more stuff lying around, but I'm going to stop for now so Ryan can go ahead and do his thing with adding queries and stuff. I hope I didn't break anything. 😅After this I'm going to try and systematise the improvements a little bit, for which our notes document should provide a good start.

[epost]

thanks for your comments @marcosh! It's nice to know there's an extra pair of eyes. It would be a good idea to see if we can get some tool to do some of these things automatically, although with the alignment and stuff, which I think is quite valuable, it may still involve a bit of manual labour. Maybe something we could look into next week.

[wisnesky]

I think you’re right that the cast only has to be evaluated once. The intention of this function is indeed to check membership in a list up to ‘heterogeneous’ equality.

On Oct 20, 2018, at 12:07 PM, Erik Post notifications@github.com wrote:

@epost commented on this pull request.

In src/Language/Mapping.hs:

@@ -141,16 +146,14 @@ conv'' ((ty2,ty):tl) = case cast ty :: Maybe ty of Nothing -> Left $ "Not in source schema/typeside: " ++ show ty2 Nothing -> Left $ "Not in target schema/typeside: " ++ show ty

-cast' :: (Typeable x, Typeable y) => x -> String -> Err y -cast' x s = case cast x of

• Nothing -> Left s
• Just y -> return y

• elem' :: (Typeable t, Typeable a, Eq a) => t -> [a] -> Bool elem' _ [] = False

@marcosh in fact, the (aligned) original impl is:

elem' :: (Typeable t, Typeable a, Eq a) => t -> [a] -> Bool

elem' _ [] = False

elem' x (y :ys) = case cast x of

Nothing -> elem' x ys

Just x' -> x' == y || elem' x ys But when elem' is invoked, cast x will be evaluated once; after that, it is passed down recursively, but unchanged. If this first evaluation of cast x gives Nothing, can the rest of the computation ever return True? It seems like this would be an equivalent fail-fast implementation:

elem' :: (Typeable t, Typeable a, Eq a) => t -> [a] -> Bool

elem' x ys = maybe False (\x' -> foldl (\acc y -> acc || y == x') False ys) (cast x) /cc @wisnesky

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.