To expand on this:
1. It's better to use when (or when-not) if one branch of your if is
just a false value. E.g. you could replace (if (empty? x) false
(whatever)) with (when-not (empty? x)). However...
2. Don't use empty? if you can help it! The idiomatic way to test
whether a collection has any items in it is to use (seq coll), which
returns nil if coll is empty, or a seq of the collection otherwise. So
then your if reduces to the very common (when (seq x) (whatever))
3. Lisp paren style is very different from C-like brace style. There
are a few rebels out there who do things differently, and if you're
the only one using the code, feel free. However, you'll get better at
reading and writing Lisp code if you let the indentation be your
guide. Parentheses are just there for the compiler; indentation is so
*you* know what's going on. There should "never" (I'm sure someone can
think of an exception) be whitespace of any kind immediately preceding
a closing bracket. Songoku's sample uses them in the traditional
style.
4. The usual indentation is two spaces, not four.
5. This last one you might already know, but it's hard to tell because
you used indents of four. Typically you indent "flow control" sorts of
statements with the standard indent, like:
(when (seq x)
(whatever))
But other sorts of multi-form constructs you usually line the
arguments up with each other: this helps a lot with tip (3), because
it means you don't need the parens to see what's up. Like so (don't
worry about what the functions do, it's just an indentation example):
(map #(+ %2 (/ 2 %1))
(filter even?
(range 4 99))
(drop-while #(< 1000)
(range)))
This way you can tell at a glance what forms fit where, even in my
nonsensical example where it's not clear what the overall purpose is.
On Oct 21, 4:12 am, Michael Gardner <gardne...@gmail.com> wrote:
> On Oct 21, 2010, at 3:28 AM, Brody Berg wrote:
>
> > (if (empty? m_list)
> > false
> > (binary-search m_list 0 (- (count m_list) 1) target))
>
> Assuming that returning nil is OK in case of the target not being present,
> you can replace this with (when (seq m_list) (binary-search ...)).
>
> > (if (== (nth m_list m_left) target)
> > (nth m_list m_left)
> > false
>
> Can similarly replace with (when (== (nth m_list m_left) target) target). But
> why are you returning the target value on success? Presumably the caller
> knows what that value is, and it makes your function awkward to use if target
> is false or nil. Why not return either the target's index, or just 'true' if
> you don't need that?
>
> Also, can you assume m_list is a vector, or if not, make it into one in your
> helper body? That would let you write (m-list n) instead of (nth m-list n),
> and would guarantee constant-time performance when you do it.
>
> > (- middle 1)
> > (+ middle 1)
>
> Can replace with (dec middle) and (inc middle), respectively.
>
> Lastly, as a matter of style: Lisp users generally hate trailing parentheses.
> It's extremely common to simply collapse them onto the previous line. Your
> editor should be able to do the grunt-work of helping you balance them, and
> they pretty much disappear after a while.
--
You received this message because you are subscribed to the Google
Groups "Clojure" group.
To post to this group, send email to clojure@googlegroups.com
Note that posts from new members are moderated - please be patient with your
first post.
To unsubscribe from this group, send email to
clojure+unsubscr...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/clojure?hl=en
