7 ms·
I never liked encountering code that chains functions calls together like this email.bulkSend(generateExpiryEmails(getExpiredUsers(db.getUsers(), Date.now())))
by hackthemack 11mo ago
I never liked encountering code that chains functions calls together like this
email.bulkSend(generateExpiryEmails(getExpiredUsers(db.getUsers(), Date.now())));
Many times, it has confused my co-workers when an error creeps in in regards to where is the error happening and why? Of course, this could just be because I have always worked with low effort co-workers, hard to say.
I have to wonder if programming should have kept pascals distinction between functions that only return one thing and procedures that go off and manipulate other things and do not give a return value.
https://docs.pascal65.org/en/latest/langref/funcproc/ https://docs.pascal65.org/en/latest/langref/funcproc/
- POiNTx 11mo agoIn Elixir this would be written as: db.getUsers() |> getExpiredUsers(Date.now()) |> generateExpiryEmails() |> email.bulkSend() I think Elixir hits the nail on the head when it comes to finding the right balance between functional and imperative style code.
- montebicyclelo 11mo agobulk_send( generate_expiry_email(user) for user in db.getUsers() if is_expired(user, date.now()) ) (...Just another flavour of syntax to look at)
- Akronymus 11mo agoNot sure I like how the binding works for user in this example, but tbh, I don't really have any better idea. Writing custom monad syntax is definitely quite a nice benefit of functional languages IMO.
- whichdan 11mo agoThe nice thing with the Elixir example is that you can easily `tap()` to inspect how the data looks at any point in the pipeline. You can also easily insert steps into the pipeline, or reuse pipeline steps. And due to the way modules are usually organized, it would more realistically read like this, if we were in a BulkEmails module: Users.all() |> Enum.filter(&Users.is_expired?(&1, Date.utc_today())) |> Enum.map(&generate_expiry_email/1) |> tap(&IO.inspect(label: "Expiry Email")) |> Enum.reject(&is_nil/1) |> bulk_send() The nice thing here is that we can easily log to the console, and also filter out nil expiry emails. In production code, `generate_expiry_email/1` would likely return a Result (a tuple of `{:ok, email}` or `{:error, reason}`), so we could complicate this a bit further and collect the errors to send to a logger, or to update some flag in the db. It just becomes so easy to incrementally add functionality here. --- Quick syntax reference for anyone reading: - Pipelines apply the previous result as the first argument of the next function - The `/1` after a function name indicates the arity, since Elixir supports multiple dispatch - `&fun/1` expands to `fn arg -> fun(arg) end` - `&fun(&1, "something")` expands to `fn arg -> fun(arg, "something") end`
- time4tea 11mo agoNot a single person in this thread commented on the use of Date.now() and similar - surely clock.now() - you never ever want to use global time in any code, how could you test it? clock in this case is a thing that was supplied to the class or function. It could just be a function: () -> Instant. (Setting a global mock clock is too evil, so don't suggest that!)
- POiNTx 11mo agoI was just referring to how pipes make these kinds of chained function calls more readable. But on your point, I think using Date.now() is perfectly ok.
- vlovich123 11mo agoWhat happens during a daylight savings adjustment?
- POiNTx 11mo agoYou use UTC which doesn't adjust daylight savings.
- ruszki 11mo ago> I think using Date.now() is perfectly ok. This is why we have tests which we need to update every 3 months, because somebody said this. This is of course, after a ton of research went into finding out why the heck our tests broke suddenly.
- fedlarm 11mo agoYou could write the logic in a more straight forward, but less composable way, so that all the logic resides in one pure function. This way you can also keep the code to only loop over the users once. email.sendBulk(generateExpiryEmails(db.getUsers(), Date.now()));
- tadfisher 11mo agoThat's pretty hardcore, like you want to restrict the runtime substitution of function calls with their result values? Even Haskell doesn't go that far. Generally you'd distinguish which function call introduces the error with the function call stack, which would include the location of each function's call-site, so maybe the "low-effort" label is accurate. But I could see a benefit in immediately knowing which functions are "pure" and "impure" in terms of manipulating non-local state. I don't think it changes any runtime behavior whatsoever, really, unless your runtime schedules function calls on an async queue and relies on the order in code for some reason. My verdict is, "IDK", but worth investigating!
- hackthemack 11mo agoIt has been so long since I worked on the code that had chaining functions and caused problems that I am not sure I can do justice to describing the problems. I vaguely remember the problem was one function returned a very structured array dealing with regex matches. But there was something wrong with the regex where once in a blue moon, it returned something odd. So, the chained functions did not error. It just did something weird. Whenever weird problems would pop up, it was always passed to me. And when I looked at it, I said, well... I am going to rewrite this chain into steps and debug each return. Then run through many different scenarios and that was how I figured out the regex was not quite correct.
- mrkeen 11mo ago> you want to restrict the runtime substitution of function calls with their result values? I don't get how you got there from parent comment. Pascal just went with a needless syntax split of (side-effectful) methods and (side-effectful) functions.
- sfn42 11mo agoI would have written each statement on its own line: var users = db.getUsers(); var expiredUsers = getExpiredUsers(users, Date.now()); var expiryEmails = generateExpiryEmails(expiredUsers); email.bulkSend(expiryEmails); This is not only much easier to read, it's also easier to follow in a stack trace and it's easier to debug. IMO it's just flat out better unless you're code golfing. I'd also combine the first two steps by creating a DB query that just gets expired users directly rather than fetching all users and filtering them in memory: expiredUsers = db.getExpiredUsers(Date.now()); Now I'm probably mostly getting zero or a few users rather than thousands or millions.
- hackthemack 11mo agoYeah. I did not mention what I would do, but what you wrote is pretty much what I prefer. I guess nobody likes it these days because it is old procedural style.
- bccdee 11mo agoThere's nothing procedural about binding return values to variables, so long as you aren't mutating them. Every functional language lets you do that. That's `let ... in` in Haskell.
- ajusa 11mo ago(author here) This is actually closer to the way the first draft of this article was written. Unfortunately, some readability was lost to make it fit on a single page. 100% agree that a statement like this is harder to reason about and should be broken up into multiple statements or chained to be on multiple lines.
- codazoda 11mo agoGlad to see this. This style seems like it’s out of vogue now, but I find it much, much easier to reason about.
- rifty 11mo agoI agree because it reads as it will process in the direction I normally read. But I do think one of the benefits of the function approach is that the scope isn't cluttered with staging variables. For these reasons one of the things I like to do in Swift is set up a function called ƒ that takes a single closure parameter. This is super minimal because Swift doesn't require parenthesis for the trailing closure. It allows me to do the above inline without cluttering the scope while also not increasing the amount of redirection using discrete function declarations would cause. The above then just looks like this: ƒ { var users = db.getUsers(); var expiredUsers = getExpiredUsers(users, Date.now()); var expiryEmails = generateExpiryEmails(expiredUsers);\ email.bulkSend(expiryEmails); }
- HiPhish 11mo ago> email.bulkSend(generateExpiryEmails(getExpiredUsers(db.getUsers(), Date.now()))); What makes it hard to reason about is that your code is one-dimensional, you have functions like `getExpiredUsers` and `generateExpiryEmails` which could be expressed as composition of more general functions. Here is how I would have written it in JavaScript: const emails = db.getUsers() .filter(user => user.isExpired(Date.now())) // Some property every user has .map(generateExpiryEmail); // Maps a single user to a message email.bulkSend(emails); The idea is that you have small but general functions, methods and properties and then use higher-order functions and methods to compose them on the fly. This makes the code two-dimensional. The outer dimension (`filter` and `map`) tells the reader what is done (take all users, pick out only some, then turn each one into something else) while the outer dimension tells you how it is done. Note that there is no function `getExpiredUsers` that receives all users, instead there is a simple and more general `isExpired` method which is combined with `filter` to get the same result. In a functional language with pipes it could be written in an arguably even more elegant design: db.getUsers() |> filter(User.isExpired(Date.now()) |> map(generateExpiryEmail) |> email.bulkSend I also like Python's generator expressions which can express `map` and `filter` as a single expression: email.bulk_send(generate_expiry_email(user) for user in db.get_users() if user.is_expired(Date.now())
- hackthemack 11mo agoI guess I just never encounter code like this in the big enterprise code bases I have had to weed through. Question. If you want to do one email for expired users and another for non expired users and another email for users that somehow have a date problem in their data.... Do you just do the const emails = three different times? In my coding world it looks a lot like doing a SELECT * ON users WHERE isExpired < Date.now but in some cases you just grab it all, loop through it all, and do little switches to do different things based on different isExpired.
- rahimnathwani 11mo agoIf you want to do one email for expired users and another for non expired users and another email for users that somehow have a date problem in their data.... Well, in that case you wouldn't want to pipe them all through generateExpiryEmail. But perhaps you can write a more generic function like generateExpiryEmailOrWhatever that understands the user object and contains the logic for what type of email to draft. It might need to output some flag if, for a particular user, there is no need to send an email. Then you could add a filter before the final (send) step.
- solid_fuel 11mo agoI may have gotten nerd sniped here, but I believe all of these examples so far have some subtle errors. Using elixir syntax, I would think something like this covers most of the cases: expiry_date = DateTime.now!("Etc/UTC") query = from u in User, where: u.expiry_date > ^expiry_date and u.expiry_email_sent == false, select: u MyAppRepo.all(query) |> Enum.map(u, &generate_expiry_emails(&1, expiry_date)) |> Email.bulkSend() # Returns {:ok, %User{}} or {:err, _reason} |> Enum.filter(fn {:ok, _} -> true _ -> false end) |> Enum.map(fn {:ok, user} -> User.changeset(user, %{expiry_email_sent: true}) |> Repo.update() end) Mainly a lot of these examples do the expiry filtering on the application side instead of the database side, and most would send expiry emails multiple times which may or may not be desired behavior, but definitely isn't the best behavior if you automatically rerun this job when it fails. ---- Edit: I actually see a few problems with this, too, since Email.bulkSend probably shouldn't know about which user each email is for. I always see a small impedance mismatch with this sort of pipeline, since if we sent the emails individually it would be easy to wrap it in a small function that passes the user through on failure. If I were going to build a user contacting system like this I would probably want a separate table tracking emails sent, and I think that the email generation could be made pure, the function which actually sends email should probably update a record including a unique email_type id and a date last sent, providing an interface like: `send_email(user_query, email_id, email_template_function)`
- lmm 11mo ago> I have to wonder if programming should have kept pascals distinction between functions that only return one thing and procedures that go off and manipulate other things and do not give a return value. What you want is to use a language that has higher-kinded types and monads so that functions can have both effects (even multiple distinct kinds of effects) and return values, but the distinction between the two is clear, and when composing effectful functions you have to be explicit about how they compose. (You can still say "run these three possibly-erroring functions in a pipeline and return either the successful result or an error from whichever one failed", but you have to make a deliberate choice to).
- Warwolt 11mo agoMaking a distinction between pure and effectful functions doesnt require any kind of effect system though. Having a language where "func" defines a pure function and "proc" defines a procedure that can performed arbitrary side effects (as in any imperative language really) would still be really useful, I think
- lmm 11mo ago> Having a language where "func" defines a pure function and "proc" defines a procedure that can performed arbitrary side effects (as in any imperative language really) would still be really useful, I think Rust tried that in the early days, the problem is no-one can agree on exactly what side effects make a function non-pure. You pay almost all the costs of a full effect system (and even have to add an extra language keyword) but get only some of the benefits.
- cestith 11mo agoThe definition I’ve used for my own projects is that anything that touches anything outside the function or in any way outlives the function is impure. It works pretty well for me. That is, no i/o, mutability of a function-local variable is okay but no touching other memory state (and that variable cannot outlive the return), the same function on the same input always produces the same output, and there’s no calling of impure code from within pure code. Notice this makes closures and currying impure unless done explicitly during function instantiation, making those things at least nominally part of the input syntactically. YMMV.
- sandeepkd 11mo agoOn the same page here, read it multiple times to see if I can convince my mind, this is bit off in terms of reading the code as its being executed. There are high chances of people making mistakes over the time with such patterns. As usual there is always a trade off involved, readability is the one taking hit here.
- tags2k 11mo agoSince everyone's giving !opinions, in my C# DDD world you'd ideally be able to: _unitOfWork.Begin(); var users = await _usersRepo.Load(u => u.LastLogin <= whateverDate); users.CheckForExpiry(); _unitOfWork.Commit(); That then writes the "send expiry email" commands from the aggregate, to an outbox, which a worker then picks up to send. Simple, transactional domain logic.
- Antibabelic 11mo agoAda is a great modern language that preserves the distinction between functions and procedures that you mention.
- MarkMarine 11mo agoThese chains become easy to read and understand with a small language feature like the pipe operator (elixir) or threading macro (clojure) that takes the output of one line and injects it into the left or rightmost function parameter. For example: (Elixir) "go " |> String.duplicate(3) # "go go go " |> String.upcase() # "GO GO GO " |> String.replace_suffix(" ", "!") # "GO GO GO!" (Clojure) ;; Nested function calls (map double (filter even? '(1 2 3 4))) ;; Using the thread-last macro (->> '(1 2 3 4) (filter even?) ; The list is passed as the last argument (map double)) ; The result of filter is passed as the last argument ;=> (4.0 8.0) Things like this have been added to python via a library (Pipe) [1] and there is a proposal to add this to JavaScript [2] 1: https://pypi.org/project/pipe/ https://pypi.org/project/pipe/ 2: https://github.com/tc39/proposal-pipeline-operator https://github.com/tc39/proposal-pipeline-operator
- netdevphoenix 11mo agoIf you get an exception, you might not know where it comes from unless you get a stack trace. Code looks nice but not practical imo
- MarkMarine 11mo agoI use Clojure all the time and I haven’t noticed the gripe you’ve got, but these are built in features of (somewhat) popular programming languages. Might not be for you but functional programming isn’t for everyone.
- shortrounddev2 11mo agoIt also invites exceptions as error handling instead of a monadic (result) pattern. I usually do something more like Result<Users> userRes = getExpiredUsers(db); if(isError(userRes)) { return userRes.error; } /* This probably wouldn't actually need to return a Result IRL */ Result<Email> emailRes = generateExpireyEmails(userRes.value); if(isError(emailRes)) { return emailRes.error; } Result<SendResult> sendRes = sendEmails(emailRes.value); if(isError(sendRes)) { return sendRes.error; } return sendRes; // successful value, or just return a Unit type. This is in my "functional C++" style, but you can write pipe helpers which sort of do the same thing: Result<SendResult> result = pipe(getExpiredUsers(db)) .then(generateExpireyEmails) .then(sendEmails) .result(); if(isError(result)) { return result.error; } If an error result is returned by any of the functions, it terminates immediately and returns the error there. You can write this in most languages, even imperative/oop languages. In java, they have a built in class called Optional with options to treat null returns as empty: Optional.ofNullable(getExpiredUsers(db)) .map(EmailService::generateExpireyEmails) .map(EmailService::sendEmails) .orElse(null); or something close to that, I haven't used java in a couple years. C++ also added a std::expected type in C++23: auto result = some_expected() .and_then(another_expected) .and_then(third_expected) .transform(/* ... some function here, I'm not familiar with the syntax*/);