Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I've written the following simple filter in Racket as my first Racket program and am wondering if I am writing it in an "idiomatic Racket style".

#! /usr/bin/env racket
#lang racket

(require racket/cmdline)

(define a-prolog-mode? (make-parameter #f))

;; parses the options passed on the command-line
(command-line
 #:program "patoms"
 #:once-any
 [("-a" "--aprolog") "output as A-Prolog code"
                     (a-prolog-mode? #t)])

;; dlv-input? : string -> boolean
;; Returns True if the given text corresponds to the output of DLV
;; and False otherwise. DLV's output is prefixed by one of the following
;; strings: "DLV", "{", or "Best model".
(define (dlv-input? text)
  (regexp-match? #rx"^DLV|^{|^Best model" text))

;; text->answer-sets : string -> list of strings
;; Returns a list comprised of all of the answer sets in the given text.
(define (text->answer-sets text)
  (cond
    [(dlv-input? text) (regexp-match* #rx"{(.*?)}" text #:match-select cadr)]
    [else null]))

;; write-as-code : string
;; Writes the given answer set in plain text form to standard output.
(define (write-as-text answer-set)
  (let ([literals (map string-trim (string-split answer-set ","))])
    (cond
      [(empty? literals) (printf "~a~n" "{}")]
      [else (for-each (λ (literal) (printf "~a~n" literal)) literals)])
    (printf "~a~n" "::endmodel")))

;; write-as-code : string
;; Writes the given answer set as A-Prolog code to standard output.
(define (write-as-code answer-set)
  (let ([literals (map string-trim (string-split answer-set ","))])
    (for-each (λ (literal) (printf "~a.~n" literal)) literals)
    (printf "~a~n" "%%endmodel")))

;; main
;; Serves as the main function of the program. If a-prolog-mode is specified
;; by the user via the command-line, all of the answer sets that may be parsed
;; from standard input are written to standard output as A-Prolog. Otherwise
;; the answer sets are written in a plain text format.
(define (main)
  (let* ([text (port->string (current-input-port))]
         [answer-sets (text->answer-sets text)])
    (cond
      [(a-prolog-mode?) (for-each write-as-code answer-sets)]
      [else (for-each write-as-code answer-sets)])))

(main)

In general, any and all feedback would be very much appreciated given that this is my first foray into Racket.

Thank you kindly in advance.

share|improve this question

1 Answer 1

up vote 1 down vote accepted

Overall, this is very pleasant code to review. Especially given you're new to the language. Your function names are follow convention such as the use of ending predicates with ? and indicating conversions with ->. The comments make this code easy to understand. So bravo, keep at it! That being said, here are some pretty minor suggestions.

  1. "Favor define when feasible". Your let's and let*'s could be changed to internal define's to decrease nesting.
  2. Change (define (main) ...) to (module+ main ...). Submodule support added in June's 5.3 release make having main's and test's easier than ever.
  3. Within main, should one of the cond branches be using write-as-text instead of write-as-code? right now, both branches do the same thing. Along the same vein, the comment above write-as-text was copied but not changed from ;; write-as-code.
  4. In racket, we have 3 ways of doing output, display, write, and print. Since your write-as-text/code functions use printing, I would change the names of those functions to be called print-as-text/code
share|improve this answer
    
Thank you very much for the feedback. I have one more question - would it be better (or more idiomatic) to have a function which returns the appropriate writer (write-as-code or write-as-text) depending on the value of a-prolog-mode? This would seem to enable me eliminate the cond in the main module, but may increase some complexity. Is there a good rule of thumb regarding this? –  ggelfond Nov 29 '12 at 20:18
    
Having a function like (define (write-answer-set answer-set) ((if (a-prolog-mode?) write-as-code write-as-text) answer-set)) isn't a bad idea. Modules that define parameters often have helper functions that consult their defined parameter to "do the right thing" so that you don't have to. –  Martin Neal Nov 29 '12 at 22:23

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.