On 11 February 2011 20:53, Nick Wiedenbrueck <nick.wiedenbru...@googlemail.com> wrote: > I'm just starting to get into Clojure. After implementing some > examples from various tutorials, I've tried to implement the following > simple server application. Now I'd like you to have a look at the code > and hear your opinion. What could be improved? What would be a more > idiomatic way implement it? Is there something that is completely non- > idiomatic? > > Here's the code: > https://github.com/cretzel/cretzel-clojure-sink/blob/0e3b33da7036b77c85393ea93bbc945ebb6dbf96/server.clj
There are a few things... 1. I'd recommend using a Clojure build tool like Leiningen (https://github.com/technomancy/leiningen). The "lein new <project-name>" command will create a blank project with sensible directory structure. 2. It's a good idea to put your code in a namespace. You can do this with the ns macro, which will also handle imports, requires, etc. For example: (ns clojure-sink.server (:import [java.net ServerSocket InetSocketAddress])) 3. Don't use camel-cased variables names. Separate the words with dashes, e.g. send-message. 4. Idiomatic Clojure code should not have hanging parenthesis. You should rely on the indentation when reading the code. e.g. (loop [] (let [clientSocket (.accept serverSocket)] (println "Client connected") (.start (Thread. #(chat clientSocket))) (recur)))) 5. Generally speaking, use two spaces for indentation, rather than tabs. A good editor will automatically indent your code like this for you. 6. The clojure.java.io namespace contains useful functions for managing the Java IO classes. You can create Reader and Writer classes directly from sockets with the reader and writer functions: (let [in (reader client-socket) out (writer client-socket)] 7. The with-open macro acts like a "let", but will call the close method on each of its values. So improving it further: (with-open [in (reader client-socket) out (writer client-socket)] 8. Rather than using "cond", you could just use an "if" 9. You could use the "while" macro instead of loop/recur. I think that's everything. - James -- 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
