1

I am trying to make a function that takes a file path and returns its content as a string. Here is my code snippet:

(defn get-string-from-file-path [path]
  (let [fs (node/require "fs") c (chan)]
      (.readFile fs path "utf8" (fn [err data] ((go (>! c (str data))))))
      (go (def r (str (<! c)))) ; is using def correct here?
       r))
  1. Is using def correct/idiomatic on line 4?
  2. After calling this function with a valid file path, I get the following error:

TypeError: (intermediate value)(intermediate value)(intermediate value)(...).call is not a function at repl:99:6 at tryToString (fs.js:414:3) at FSReqWrap.readFileAfterClose [as oncomplete] (fs.js:401:12)

Weirdly enough, if I call the function again I get the desired string content of the file! What is going on here?

EDIT: Here is the corrected function:

(defn get-string-channel-from-file-path [path] ; function is now "get string channel" instead of "get string"
  (let [fs (node/require "fs") c (chan)]
      (.readFile fs path "utf8" (fn [err data] (go (>! c data))))
        c)) ; return channel instead of string
George
  • 6,927
  • 4
  • 34
  • 67

1 Answers1

1

  1. No, using def anywhere but at the root scope of your source is generally a bad idea and not idiomatic. If you want to create another local variable then another let expression is the way to go.

  2. For why its working sometimes, you are taking an async function and trying to make it synchronous. This is leading to race conditions. Sometimes the File System is returning the file before you return from the function (and that is why works) and sometimes it is not (error).

There are a few options:

  • You can try to make the function synchronous by using node's readFileSync function. That way you can just return what it returns.

  • Instead of trying to extract the data in the go block, you should return the chan you are putting to >! and have the next function get the data from the channel in its own go block <!. This is much more idiomatic than callbacks when using core.async.

rabidpraxis
  • 556
  • 3
  • 10
  • `readFileSync` is the easiest solution. For learning purposes, however, I corrected the function above which continues to use the asynchronous `readFile`. Now the function returns a channel instead of a string. Does it look correct and idiomatic to you? – George Jan 01 '16 at 16:13
  • 1
    It looks much better yes! There are a couple small things: You don't need the extra parens around the go expression. Also the contents of data is a string, so the call `(str data)` I don't believe is doing anything. – rabidpraxis Jan 01 '16 at 16:16
  • Cool. I corrected the function above. But also: I feel like my function naming isn't idiomatic because it uses the word "get" and refers to the types "chan" and "string" (hence the name feels more idiomatic to a language like Java than to a language like ClojureScript). Is my intuition correct? If so, what would be a suitably idiomatic name for this sort of function? – George Jan 01 '16 at 16:39
  • Clojure functions tend to be on the terser side and usually describe the core of the function. I would say `read-file-chan` would be an acceptable function name. That includes the core of the function and the return type. I don't always add the return type to the function name, but I usually make an exception for core.async channels, as its real important you know you are getting a chan back. All that being said, this is also a personal or team based preference. This is a good resource for style questions: https://github.com/bbatsov/clojure-style-guide – rabidpraxis Jan 01 '16 at 16:55