2023-09-08 code-reviews | Clojure Slack Archive

code-reviews 2023-09-08

2023-09-08T07:24:40.044499Z

this is a footgun I discovered in my codebase. I have a need to convert date string from a DB to one timezone, and use its string result in a html. I solve that with the following code:

(def utc-formatter (.. (DateTimeFormatter/ofPattern "yyyy-MM-dd HH:mm:ss")
                       (withZone (ZoneId/of "UTC"))))
(def jakarta-formatter (.. utc-formatter (withZone (ZoneId/of "Asia/Jakarta"))))

(defn timezone-utc->jkt [time-str]
  (->> time-str (.parse utc-formatter) (.format jakarta-formatter)))

this has been working quite well until just recently become a source of confusion. I have “fixed” it for now, but I got an idea for this channel: I want this small code to be reviewed. I heard that the smaller the code, the fiercer the review 😁, so what is problematic with this code and how would you deal with it? (asking this in the spirit of comparing answer that I’ve come up myself)

Fahd El Mazouni 2023-09-11T08:09:55.639769Z

what have you come up with ? 😛 at first glace I'd say I wouldn't define the formatters in case they're stateful, but after having a look at the docs I don't think it's the source of your problem

2023-09-11T08:46:56.596539Z

the confusing error was actually a NullPointerException and I modified it to become

(defn timezone-utc->jkt [time-str]
  (or (some->> time-str (.parse utc-formatter) (.format jakarta-formatter))
      "nil time"))

I felt kinda silly because the source of this error should have been discovered quickly had I have the exception trace info but this problem also led me to discover that some server middleware I set up wrongly erase the exception stacktrace. So it was especially confusing because there error is simply says`java.util.Objects requireNonNull Objects.java 233` while I have made sure that there is no null/blank values in the database, and there is another part of the code that use the same code but working just fine. after some manual tracing of the code, I found that timezone-utc->jkt was the one that threw the NullPointerException, the nil value was actually caused by a qualified keyword typo in map destructuring somewhere in my code.

Fahd El Mazouni 2023-09-11T09:00:32.339819Z

so basically time-str was read wrong and you were passing a nil value ? in my opinion the bug is the typo, or whatever caused the value to be nil in the first place. I guess it comes down to preference ? but I prefer having the code crash if it gets something unexpected

2023-09-11T09:56:32.205979Z

I think I agree with your perspective, since I have fixed the middleware that gobbled up the exception information, it seems better to revert that function back and let the exception thrown. chronologically I came up with the some-> solution before realizing the middleware issue after all

👍 1

Clojurians Log v2

code-reviews 2023-09-08