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:
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.
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?".
CodePudding user response:
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
- it needs to access locally bound variables. In this case it must be local, or else you need extra parameters.
- 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.
CodePudding user response:
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 where
s; 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'
]