On Jan 18, 2011, at 10:59 AM, Tim Visher wrote:
> I'd love to have the code torn apart a little bit and get some
> suggestions for improvements.
>
> It's located at https://gist.github.com/784744
It took me a few tries to figure out what to-wallpaper-name was doing. Maybe
you should write a helper called split-filename or such, which you could use
like (let [[base extension] (split-filename name)] ...). Also, why bother to
negate the result of the name-has-resolution? test when you're going to do
something in both cases anyway?
Why are you constructing your wallpaper structs in two steps (first creating
without-destination-file and then assoc-ing the destination file to it)?
I'd chop the "to-" prefix off the front of most of your function names, as it
feels redundant to me.
I'd also use map instead of for in wallpaper-resolutions, since it reads better
to me with the list of resolutions coming at the end:
(map (partial apply struct resolution)
[[1280 1024] [1920 1080] [2560 1600] [1920 1200] [1680 1050] [1440 900]
[1280 800]])
Actually, you might consider just using 2-vectors for resolutions instead of
structs/records. Besides being simpler, it would allow you to do things like
(clojure.string/join "x" my-resolution).
--
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
