Clojure

when-first double evaluation

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Completed
  • Affects Version/s: Release 1.4
  • Fix Version/s: Release 1.5
  • Component/s: None
  • Labels:
  • Environment:
    Mac OS X 10.8.1, Java 1.7.0_06
  • Patch:
    Code
  • Approval:
    Ok

Description

The when-first macro will evaluate the xs expression twice. Admittedly, it does exactly what the doc string says, but that seems undesirable to me. Even without side effects, there's a potential performance issue if xs is some expensive operation.

Patch attached. The main diff is:

- `(when (seq ~xs)
- (let [~x (first ~xs)]
- ~@body))))
+ `(when-let xs# (seq ~xs)
+ (let ~x (first xs#)
+ ~@body))))

Activity

Steve Miner made changes -
Field Original Value New Value
Description The when-first macro will evaluate the xs expression twice. Admittedly, it does exactly what the doc string says, but that seems undesirable to me. Even without side effects, there's a potential performance issue if xs is some expensive operation.

Patch attached. The main diff is:

- `(when (seq ~xs)
- (let [~x (first ~xs)]
- ~@body))))
+ `(when-let [xs# (seq ~xs)]
+ (let [~x (first xs#)]
+ ~@body))))
The when-first macro will evaluate the xs expression twice. Admittedly, it does exactly what the doc string says, but that seems undesirable to me. Even without side effects, there's a potential performance issue if xs is some expensive operation.

Patch attached. The main diff is:

\- `(when (seq ~xs)
\- (let [~x (first ~xs)]
\- ~@body))))
+ `(when-let [xs# (seq ~xs)]
+ (let [~x (first xs#)]
+ ~@body))))
Stuart Sierra made changes -
Assignee Stuart Sierra [ stuart.sierra ]
Stuart Sierra made changes -
Waiting On richhickey
Approval Screened [ 10004 ]
Rich Hickey made changes -
Approval Screened [ 10004 ] Ok [ 10007 ]
Fix Version/s Release 1.5 [ 10150 ]
Stuart Halloway made changes -
Resolution Completed [ 1 ]
Status Open [ 1 ] Closed [ 6 ]

People

Vote (1)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: