Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: transform-exactly-one #172

Closed
mattiasw2 opened this issue Feb 9, 2017 · 4 comments
Closed

Feature request: transform-exactly-one #172

mattiasw2 opened this issue Feb 9, 2017 · 4 comments

Comments

@mattiasw2
Copy link

mattiasw2 commented Feb 9, 2017

During enhancement of the system, it is easy to forget some places where changes are needed.

I use transform a lot, however, maybe I forgot to update the path.

Having a transform-one!, which similar to select-one! would throw an exception if no changes, or too many changes are made, would be helpful.

@mattiasw2
Copy link
Author

mattiasw2 commented Feb 9, 2017

I assume this is as efficient as it can be?

(defn compiled-transform-exactly-one*
  [apath transform-fn structure]
  (let [cnt (atom 0)
        transform-fn1 (fn [x]
                        (swap! cnt inc)
                        (transform-fn x))
        res (com.rpl.specter.impl/compiled-transform* apath transform-fn1 structure)]
    (if (not= 1 @cnt) (com.rpl.specter.impl/throw-illegal (str @cnt " element(s) found in structure: ") structure))
    res))


(defmacro transform-one!
  "Navigates to exactly one value specified by the path and replaces it by the result of running the transform-fn on it, otherwise throw an exception.
   This macro will do inline caching of the path."
  [apath transform-fn structure]
  `(compiled-transform-exactly-one* (sp/path ~apath) ~transform-fn ~structure))))

@nathanmarz
Copy link
Collaborator

Your implementation will be more efficient by using com.rpl.specter.impl/mutable-cell rather than atom. I'll have to think more about whether it's worth adding this into Specter core, so I'll leave the issue open for the time being.

@mattiasw2
Copy link
Author

Updated code

  (defn compiled-transform-one!*
    [apath transform-fn structure]
    (let [cnt (com.rpl.specter.impl/mutable-cell 0)
          transform-fn1 (fn [x]
                          (com.rpl.specter.impl/update-cell! cnt inc)
                          (transform-fn x))
          res (com.rpl.specter.impl/compiled-transform* apath transform-fn1 structure)]
      (if (not= 1 (com.rpl.specter.impl/get-cell cnt)) (com.rpl.specter.impl/throw-illegal (str @cnt " element(s) found in structure: ") structure))
      res))
  
  
  (defmacro transform-one!
    "Navigates to exactly one value specified by the path and replaces it by the result of running the transform-fn on it, otherwise throw an exception.
     This macro will do inline caching of the path."
    [apath transform-fn structure]
    `(compiled-transform-one!* (com.rpl.specter/path ~apath) ~transform-fn ~structure))

@nathanmarz
Copy link
Collaborator

Thought about it and decided not to add this to Specter. From my experience I just don't see it being that valuable, and adding it could create confusion among some users (e.g. "Do I have to use transform-one! if it only touches one element?" or "Is it faster to use transform-one! over transform if it only touches one element?")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants