Clojure

Docstrings for `conj!` and `assoc!` should suggest using the return value; effect not always in-place

Details

  • Type: Enhancement Enhancement
  • Status: Open Open
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: Release 1.6
  • Fix Version/s: Release 1.11
  • Component/s: None
  • Patch:
    Code
  • Approval:
    Incomplete

Description

The docstrings of both `assoc!` and `conj!` say "Returns coll.", possibly suggesting the transient edit happens (always) in-place, `coll` being the first argument. However, this is not the case and the returned collection should always be the one that's used.

Approach: Replace "Returns coll." with "Returns an updated collection." in `conj!`, `assoc!`, `pop!` docstrings.

Patch: CLJ-1385-reword-docstrings-on-transient-update-funct-2.patch

Screened by: Alex Miller

Activity

Alex Miller made changes -
Field Original Value New Value
Labels collections docstring
Alex Miller made changes -
Issue Type Defect [ 1 ] Enhancement [ 4 ]
Priority Major [ 3 ] Minor [ 4 ]
Alex Miller made changes -
Description The docstrings of both `assoc!` and `conj!` say "Returns coll.", suggesting the transient edit happens always in-place, `coll` being the first argument.

However, the fact that the following example omits the key `8` in its result proves that in-place edits aren't always the case:

    (let [a (transient {})]
      (dotimes [x 9]
        (assoc! a x :ok))
      (persistent! a))
    ;;=> {0 :ok, 1 :ok, 2 :ok, 3 :ok, 4 :ok, 5 :ok, 6 :ok, 7 :ok}

Instead, programmers should be guided towards using constructs like `reduce` with transients:

    (persistent! (reduce #(assoc! %1 %2 :ok)
                 (transient {})
                 (range 9)))
    ;;=> {0 :ok, 1 :ok, 2 :ok, 3 :ok, 4 :ok, 5 :ok, 6 :ok, 7 :ok, 8 :ok}

The easiest way to achieve this is by changing the docstrings of (at least) `conj!` and `assoc!` to not read "Returns coll." but instead tell that the change is destructive.
The docstrings of both `assoc!` and `conj!` say "Returns coll.", suggesting the transient edit happens always in-place, `coll` being the first argument.

However, the fact that the following example omits the key `8` in its result proves that in-place edits aren't always the case:

{code}
    (let [a (transient {})]
      (dotimes [x 9]
        (assoc! a x :ok))
      (persistent! a))
    ;;=> {0 :ok, 1 :ok, 2 :ok, 3 :ok, 4 :ok, 5 :ok, 6 :ok, 7 :ok}
{code}

Instead, programmers should be guided towards using constructs like `reduce` with transients:

{code}
    (persistent! (reduce #(assoc! %1 %2 :ok)
                 (transient {})
                 (range 9)))
    ;;=> {0 :ok, 1 :ok, 2 :ok, 3 :ok, 4 :ok, 5 :ok, 6 :ok, 7 :ok, 8 :ok}
{code}

The easiest way to achieve this is by changing the docstrings of (at least) `conj!` and `assoc!` to not read "Returns coll." but instead tell that the change is destructive.
Gary Fredericks made changes -
Andy Fingerhut made changes -
Patch Code [ 10001 ]
Gary Fredericks made changes -
Alex Miller made changes -
Labels collections docstring collections docstring ft
Description The docstrings of both `assoc!` and `conj!` say "Returns coll.", suggesting the transient edit happens always in-place, `coll` being the first argument.

However, the fact that the following example omits the key `8` in its result proves that in-place edits aren't always the case:

{code}
    (let [a (transient {})]
      (dotimes [x 9]
        (assoc! a x :ok))
      (persistent! a))
    ;;=> {0 :ok, 1 :ok, 2 :ok, 3 :ok, 4 :ok, 5 :ok, 6 :ok, 7 :ok}
{code}

Instead, programmers should be guided towards using constructs like `reduce` with transients:

{code}
    (persistent! (reduce #(assoc! %1 %2 :ok)
                 (transient {})
                 (range 9)))
    ;;=> {0 :ok, 1 :ok, 2 :ok, 3 :ok, 4 :ok, 5 :ok, 6 :ok, 7 :ok, 8 :ok}
{code}

The easiest way to achieve this is by changing the docstrings of (at least) `conj!` and `assoc!` to not read "Returns coll." but instead tell that the change is destructive.
The docstrings of both `assoc!` and `conj!` say "Returns coll.", possibly suggesting the transient edit happens (always) in-place, `coll` being the first argument. However, this is not the case and the returned collection should always be the one that's used.

*Approach:* Replace "Returns coll." with "Returns an updated collection." in `conj!`, `assoc!`, `pop!` docstrings.

*Patch:* CLJ-1385-reword-docstrings-on-transient-update-funct-2.patch

*Screened by:* Alex Miller
Approval Triaged [ 10120 ]
Rich Hickey made changes -
Fix Version/s Release 1.8 [ 10254 ]
Approval Triaged [ 10120 ] Vetted [ 10003 ]
Alex Miller made changes -
Assignee Alex Miller [ alexmiller ]
Alex Miller made changes -
Fix Version/s Release 1.9 [ 10750 ]
Fix Version/s Release 1.8 [ 10254 ]
Alex Miller made changes -
Approval Vetted [ 10003 ] Incomplete [ 10006 ]
Alex Miller made changes -
Fix Version/s Release 1.9 [ 10750 ]
Fix Version/s Release Next [ 11451 ]
Alex Miller made changes -
Fix Version/s Release 1.10 [ 11451 ]
Fix Version/s Release 1.11 [ 11750 ]

People

Vote (5)
Watch (3)

Dates

  • Created:
    Updated: