Send Beginners mailing list submissions to
        [email protected]

To subscribe or unsubscribe via the World Wide Web, visit
        http://www.haskell.org/mailman/listinfo/beginners
or, via email, send a message with subject or body 'help' to
        [email protected]

You can reach the person managing the list at
        [email protected]

When replying, please edit your Subject line so it is more specific
than "Re: Contents of Beginners digest..."


Today's Topics:

   1.  Feedback on my first, small project (piece). (Dominik Bollmann)
   2. Re:  Feedback on my first, small project (piece). (David McBride)
   3. Re:  Feedback on my first, small project (piece). (Tony Morris)
   4. Re:  Feedback on my first, small project (piece).
      (Henk-Jan van Tuyl)


----------------------------------------------------------------------

Message: 1
Date: Tue, 05 Aug 2014 22:08:51 +0200
From: Dominik Bollmann <[email protected]>
To: [email protected]
Subject: [Haskell-beginners] Feedback on my first, small project
        (piece).
Message-ID:
        <87fvhaya0c.fsf@debox.i-did-not-set--mail-host-address--so-tickle-me>
Content-Type: text/plain


Hello Haskeller's,

I recently dived into Haskell and then wanted to practice it a bit!
Therefore I wrote a small program that matches UNIX-style globs. The
program behaves kind of like grep, just that it matches a glob and not a
regular expression. And also, it offers only *very* rudimental
functionality compared to grep.

The code is available on github: https://github.com/bollmann/Globber
When writing the program I tried to satisfy the specification as given
at: http://www.scs.stanford.edu/14sp-cs240h/labs/lab1.html. This Lab
material btw. also inspired me to try writing such a program :-).

In order to improve my programming in Haskell, I would love to hear
feedback from you guys on this first small project of mine. Any comments
regarding style, used idioms, as well as general and specific code
improvements are highly appreciated. Thanks!

Cheers, Dominik.


------------------------------

Message: 2
Date: Tue, 5 Aug 2014 16:42:39 -0400
From: David McBride <[email protected]>
To: The Haskell-Beginners Mailing List - Discussion of primarily
        beginner-level topics related to Haskell <[email protected]>
Subject: Re: [Haskell-beginners] Feedback on my first, small project
        (piece).
Message-ID:
        <CAN+Tr42M49dxkKYR78o-OG3=Qr=din5x-mgwaylncmvkvaa...@mail.gmail.com>
Content-Type: text/plain; charset="utf-8"

I took a look at your main and made some changes that might help.  I have
not looked at your glob file at all.

1.  Rather than using length args successively in multiple if/else if
statements, which recalculates the value, just use a case, calculate it
once and then go from there, when you can.

2. Use let or where to make new function rather than putting them all
inline.

3. Instead of looping the main entirely to get all of stdin, use said new
function and have it loop itself.  You could also streamline this to have
its own function that does not require the glob to be passed into each loop.

4. Use withFile instead of opening the file and then forgetting about the
handle.  If you had enough files on the commandline, you would exhaust the
allowable open file handles on the system.  Withfile will close the handle
when you are done with each one.

5. Use mapM_ to get rid of the results that are returned by mapM.  It is
also faster if you are not going to use the results anyways.

6. When you are in a monad (such as IO), you can use when instead of if to
optionally perform a statement, and it looks a bit prettier.

There are other minor things, but this is a pretty good start.

Here's the resulting code (I have not run it, but it should be pretty close
to what you had): https://gist.github.com/anonymous/615e48004ca2eed82d0a


On Tue, Aug 5, 2014 at 4:08 PM, Dominik Bollmann <[email protected]>
wrote:

>
> Hello Haskeller's,
>
> I recently dived into Haskell and then wanted to practice it a bit!
> Therefore I wrote a small program that matches UNIX-style globs. The
> program behaves kind of like grep, just that it matches a glob and not a
> regular expression. And also, it offers only *very* rudimental
> functionality compared to grep.
>
> The code is available on github: https://github.com/bollmann/Globber
> When writing the program I tried to satisfy the specification as given
> at: http://www.scs.stanford.edu/14sp-cs240h/labs/lab1.html. This Lab
> material btw. also inspired me to try writing such a program :-).
>
> In order to improve my programming in Haskell, I would love to hear
> feedback from you guys on this first small project of mine. Any comments
> regarding style, used idioms, as well as general and specific code
> improvements are highly appreciated. Thanks!
>
> Cheers, Dominik.
> _______________________________________________
> Beginners mailing list
> [email protected]
> http://www.haskell.org/mailman/listinfo/beginners
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: 
<http://www.haskell.org/pipermail/beginners/attachments/20140805/d8509743/attachment-0001.html>

------------------------------

Message: 3
Date: Wed, 06 Aug 2014 08:42:56 +1000
From: Tony Morris <[email protected]>
To: [email protected]
Subject: Re: [Haskell-beginners] Feedback on my first, small project
        (piece).
Message-ID: <[email protected]>
Content-Type: text/plain; charset="iso-8859-1"

https://gist.github.com/tonymorris/a0e82e603d4d32597bbc/revisions

On 06/08/14 06:42, David McBride wrote:
> I took a look at your main and made some changes that might help.  I
> have not looked at your glob file at all.
>
> 1.  Rather than using length args successively in multiple if/else if
> statements, which recalculates the value, just use a case, calculate
> it once and then go from there, when you can.
>
> 2. Use let or where to make new function rather than putting them all
> inline.
>
> 3. Instead of looping the main entirely to get all of stdin, use said
> new function and have it loop itself.  You could also streamline this
> to have its own function that does not require the glob to be passed
> into each loop.
>
> 4. Use withFile instead of opening the file and then forgetting about
> the handle.  If you had enough files on the commandline, you would
> exhaust the allowable open file handles on the system.  Withfile will
> close the handle when you are done with each one.
>
> 5. Use mapM_ to get rid of the results that are returned by mapM.  It
> is also faster if you are not going to use the results anyways.
>
> 6. When you are in a monad (such as IO), you can use when instead of
> if to optionally perform a statement, and it looks a bit prettier.
>
> There are other minor things, but this is a pretty good start.
>
> Here's the resulting code (I have not run it, but it should be pretty
> close to what you had):
> https://gist.github.com/anonymous/615e48004ca2eed82d0a
>
>
> On Tue, Aug 5, 2014 at 4:08 PM, Dominik Bollmann
> <[email protected] <mailto:[email protected]>> wrote:
>
>
>     Hello Haskeller's,
>
>     I recently dived into Haskell and then wanted to practice it a bit!
>     Therefore I wrote a small program that matches UNIX-style globs. The
>     program behaves kind of like grep, just that it matches a glob and
>     not a
>     regular expression. And also, it offers only *very* rudimental
>     functionality compared to grep.
>
>     The code is available on github: https://github.com/bollmann/Globber
>     When writing the program I tried to satisfy the specification as given
>     at: http://www.scs.stanford.edu/14sp-cs240h/labs/lab1.html. This Lab
>     material btw. also inspired me to try writing such a program :-).
>
>     In order to improve my programming in Haskell, I would love to hear
>     feedback from you guys on this first small project of mine. Any
>     comments
>     regarding style, used idioms, as well as general and specific code
>     improvements are highly appreciated. Thanks!
>
>     Cheers, Dominik.
>     _______________________________________________
>     Beginners mailing list
>     [email protected] <mailto:[email protected]>
>     http://www.haskell.org/mailman/listinfo/beginners
>
>
>
>
> _______________________________________________
> Beginners mailing list
> [email protected]
> http://www.haskell.org/mailman/listinfo/beginners

-------------- next part --------------
An HTML attachment was scrubbed...
URL: 
<http://www.haskell.org/pipermail/beginners/attachments/20140806/7138cb6e/attachment-0001.html>

------------------------------

Message: 4
Date: Wed, 06 Aug 2014 00:46:12 +0200
From: "Henk-Jan van Tuyl" <[email protected]>
To: [email protected], "Dominik Bollmann"
        <[email protected]>
Subject: Re: [Haskell-beginners] Feedback on my first, small project
        (piece).
Message-ID: <op.xj45jfblpz0j5l@alquantor>
Content-Type: text/plain; charset=iso-8859-15; format=flowed;
        delsp=yes

On Tue, 05 Aug 2014 22:08:51 +0200, Dominik Bollmann  
<[email protected]> wrote:

:
> The code is available on github: https://github.com/bollmann/Globber
> When writing the program I tried to satisfy the specification as given
> at: http://www.scs.stanford.edu/14sp-cs240h/labs/lab1.html. This Lab
> material btw. also inspired me to try writing such a program :-).
>
> In order to improve my programming in Haskell, I would love to hear
> feedback from you guys on this first small project of mine. Any comments
> regarding style, used idioms, as well as general and specific code
> improvements are highly appreciated. Thanks!
:

You could change
matchGlob' :: GlobPattern -> String -> Bool
matchGlob' glob string
   | null glob && null string = True
   | null glob && not (null string) = False
   | null string && glob == "*" = True
   | null string && glob /= "*" = False
matchGlob' (p:ps) (s:sx)
   | p == '?' = matchGlob' ps sx
   | p == '\\' = matchEscapedChar ps (s:sx)
   | p == '*' = matchStar ps (s:sx)
   | p == '[' = matchSet (p:ps) (s:sx)
   | otherwise = p == s && matchGlob' ps sx


to:
matchGlob' :: GlobPattern -> String -> Bool
matchGlob' []       string = null string
matchGlob' glob      []    = glob == "*"
matchGlob' ('?':ps) (s:sx) = matchGlob'       ps sx
matchGlob' ('\\':ps) sx    = matchEscapedChar ps sx
matchGlob' ('*':ps)  sx    = matchStar        ps sx
matchGlob' ('[':ps)  sx    = matchSet   ('[':ps) sx
matchGlob' (p:ps)   (s:sx) = p == s && matchGlob' ps sx

(not tested)

In function buildCharChoices, you have written
   where isRange chars = if (length chars) == 3 && (chars !! 1) == '-' then
                         True
                       else
                         False
, this can be simplified to
   where isRange chars = length chars == 3 && chars !! 1 == '-'


Regards,
Henk-Jan van Tuyl


-- 
Folding@home
What if you could share your unused computer power to help find a cure? In  
just 5 minutes you can join the world's biggest networked computer and get  
us closer sooner. Watch the video.
http://folding.stanford.edu/


http://Van.Tuyl.eu/
http://members.chello.nl/hjgtuyl/tourdemonad.html
Haskell programming
--


------------------------------

Subject: Digest Footer

_______________________________________________
Beginners mailing list
[email protected]
http://www.haskell.org/mailman/listinfo/beginners


------------------------------

End of Beginners Digest, Vol 74, Issue 3
****************************************

Reply via email to