[Haskell-cafe] Comments and suggestions on code
jonathanccast at fastmail.fm
Thu Jan 10 23:37:49 EST 2008
On 10 Jan 2008, at 10:21 AM, Andre Nathan wrote:
> Hi Jonathan
> On Wed, 2008-01-09 at 21:32 -0800, Jonathan Cast wrote:
>> An actual coding question, abuse? We should be so lucky.
> :) Your comments are much appreciated.
>> This function is fairly complicated, simply because of the number of
>> separate definitions involved; I would be looking for opportunities
>> to inline definitions here, so it's clearer what the definitions
>> are. (Also, I would try to build a single, self-recursive function
>> at the top level, put the call to procInfo there, and make everything
>> else pure).
> I rewrote insertInTree like below. Now it is the only function that
> a StateT return type, and I also got rid of addProc, insertPid and
> insertParent :)
> insertInTree :: Pid -> StateT PsTree IO ()
> insertInTree pid = do
> tree <- get
> if Map.member pid tree
> then return ()
> else do
> info <- lift $ procInfo pid
> modify (Map.insert pid info)
> let ppid = parentPid info
> if ppid /= "0"
> then do
> insertInTree ppid
> modify (appendChild ppid pid)
> else return ()
> I also rewrote createTree like this:
> createTree :: IO PsTree
> createTree = do
> entries <- getDirectoryContents "/proc"
> let procs = filter (=~ "^[0-9]+$") entries
> execStateT (mapM_ insertInTree procs) Map.empty
> Is that a bad way to do it? If haskell wasn't lazy this would be 3 O
> operations, and I could write it using readDirStream to process all
> entries in one pass. I'm not sure if that's really necessary when
> laziness is present though.
It might be faster; laziness usually has higher constants than direct
implementations. But I doubt the difference is critical in this
case, and I would definitely time a re-writing and throw it away
unless it was significantly faster. But I don't think this is a case
where laziness actually alters either the time or the space
asymptotics of the algorithm (you end up creating an ~ O(n) tree
anyway, so I'd figure O(n) space was OK for the loop, too).
More information about the Haskell-Cafe