-1

I faced with some issue in Haskell: code-style and big functions (I'm continuing to learn Haskell through writing toy-language).

I have some necessary-big-function (see example). And there are two sub-function (nested where) in it. And there is no reason to place its sub-finction in module scoupe. How "Haskell code-style" or "Haskell code-style best practics" suggest to solve this problem of "ungraceful and clumsy code"?

Function (with later comment):

-- We can delete (on DCE) any useless opers.
-- Useful opers - only opers, whitch determine (directly or transitivery) result of GlobalUse oper
addGlobalUsageStop :: [Var] -> IR -> IR
addGlobalUsageStop guses iR = set iOpers (ios ++ ios') $ set opN opn' iR
    where
    ios = _iOpers iR
    gdefs = _gDefs iR :: M.Map Int Var
    opn = _opN iR
    guses' = nub $ filter isRegGlobal guses
    ogs = catMaybes $ map (concatIOperWithGDef gdefs) $ reverse ios
        where
        concatIOperWithGDef gdefs' (i, o) = case gdefs' M.!? i of
            Nothing -> Nothing
            Just gd -> Just (o, gd)
    nops = newGUses ogs guses'
        where
        newGUses [] _ = []
        newGUses _ [] = []
        newGUses ((Oper _ d _ _, g):os) guses = if elem g guses
            then (Oper GlobalUse g (RVar d) None):newGUses os (filter (g /=) guses)
            else newGUses os guses
    ios' = zip [opn..] nops
    opn' = opn + length ios'

Notices:

  1. If you want to know why I even wrote such big function the answer is: because this is some big (and one-needed functionality in compiler): - for each "returning variable" we shoul find last operation, witch defines it (actually corresponding virtual register), and expand our IR with constructed opers.

  2. I'v seen some similar questions: Haskell nested where clause and "let ... in" syntax but they are about "how typewrite correct code?", and my question "is this code Code-Style correct, and if it isn't - what should i do?".

Improve this question

2 Answers 2

Reset to default
4

And there is no reason to place its sub-function in module scope

Think about it the other way around. Is there any reason to put the sub-function in the local scope? Then do it. This could be because

  1. it needs to access locally bound variables. In this case it must be local, or else you need extra parameters.
  2. it does something very obvious and only relevant to the specific use case. This could be one-line definitions of some operation that you don't care thinking a properly descriptive name, or it could be a go helper that does basically the whole work of the enclosing function.

If neither of these apply, and you can give the local function a descriptive name (as you've already done) then put it in module scope. Add a type signature, which makes it clearer yet. Don't export it from the module.

Putting the function in module scope also removes the need to rename variables like you did with gdefs'. That's one of the more common causes for bugs in Haskell code.

Improve this answer
Sign up to request clarification or add additional context in comments.

1 Comment

>> Putting the function in module scope also removes the need to rename variables like you did with gdefs'. That's one of the more common causes for bugs in Haskell code. -- ===================== Thank you for additionaly notice this issue. It have coused bugs already.
3

The question is a good one, but the example code isn't a great example. For me, the correct fix in this particular case is not to talk about how to stylishly nest wheres; it's to talk about how to use library functions and language features to simplify the code enough that you don't need where in the first place. In particular, list comprehensions get you very far here. Here's how I would write those two definitions:

import Data.Containers.ListUtils (nubOrdOn)

... where
    ogs = [(o, gd) | (i, o) <- reverse ios, Just gd <- [gdefs M.!? i]]
    nops = nubOrdOn fun
        [ Oper GlobalUse g (RVar d) None
        | (Oper _ d _ _, g) <- ogs
        , g `elem` guses'
        ]
    fun (Oper _ g _ _) = g -- this seems useful enough to put at the global scope; it may even already exist there

Since ogs isn't mentioned anywhere else in the code, you might consider inlining it:

    -- delete the definition of ogs
    nops = nubOrdOn fun
        [ Oper GlobalUse g (RVar d) None
        | (i, Oper _ d _ _) <- reverse ios
        , Just g <- [gdefs M.!? i]
        , g `elem` guses'
        ]
Improve this answer

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.