1305066 - Remove the Environment monad from the resolver

Status: RESOLVED FIXED

(L20n :: JS Library, defect)

Description

[Staś Małolepszy :stas]

The Environment monad was modeled after the Reader monad from Haskell.  It abstracts the action of passing additional arguments to each of the resolver's functions and makes it trivial to manage errors when they're returned in a [value, errors] tuple from each function.  It automatically handles concatenating old errors and the new ones and makes it transparent to the developer.

In bug 1301198 we moved away from returning errors explicitly and instead we push them to an array passed as an argument.  This makes the use-case for having the Environment monad less compelling.  The only benefit right now is that we don't have to pass all the arguments to each function but that could be mitigated by passing a single `env` argument.  OTOH, Environment adds complexity and has some performance impact, although it's only noticeable when formatting a lot of complex strings.

Let's remove it.

Comment 1

[Staś Małolepszy :stas]

Attachment: Pull request

Comment 2

[Zibi Braniecki [:zbraniecki][:gandalf]]

What kind of performance are we talking about?

If performance was compelling, would you prefer to keep the monad?

Comment 3

[Staś Małolepszy :stas]

No difference on workload-low, and -27% (18ms vs 13ms) on workload-high.

The monad doesn't give us much any more b/c of how we change error handling and passing an additional env object doesn't bother me too much either.  It's similar to what the parser does by creating a ParseContext if you think about it.

If we ever want to go back to returning errors or choose to do something something else entirely that would benefit from this monad (or some other), we know how to do it and we can even reuse the code (which I also documented earlier today!).

Right now my votes goes to simpler codebase.

Comment 4

[Staś Małolepszy :stas]

https://github.com/l20n/l20n.js/commit/106f6229d243b274d3301041f19e0b7bcf8bd332