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

use maybes for unsafe operations #683

Closed
davidchambers opened this Issue Dec 9, 2014 · 40 comments

Comments

Projects
None yet
7 participants
@davidchambers
Member

davidchambers commented Dec 9, 2014

Certain Ramda functions are unsafe. Take this quartet, for example:

R.head :: [a] -> a | Undefined  -- unsafe
R.last :: [a] -> a | Undefined  -- unsafe
R.tail :: [a] -> [a]            -- safe, but misleading
R.init :: [a] -> [a]            -- safe, but misleading

I suggest the following types:

R.head :: [a] -> Maybe a
R.last :: [a] -> Maybe a
R.tail :: [a] -> Maybe [a]
R.init :: [a] -> Maybe [a]

We could also provide the unsafe versions:

R.unsafeHead :: [a] -> a | Undefined
R.unsafeLast :: [a] -> a | Undefined
R.unsafeTail :: [a] -> [a]
R.unsafeInit :: [a] -> [a]

We'd also provide the following function which takes a default value as its first argument:

R.fromMaybe :: a -> Maybe a -> a

Let's consider a simple example: taking the first element of a [String] of unknown length and converting it to upper-case.

// Naive solution (will throw if list is empty)
R.toUpper(R.head(list))

// Pre-emptive check
R.ifElse(R.isEmpty, R.always('<default>'), R.pipe(R.head, R.toUpperCase))(list)

// Maybe
R.fromMaybe('<default>', R.map(R.toUpperCase, R.head(list)))

It's cleaner and less error-prone to map over a maybe than to rely on checking for the empty list whenever one uses a member of this quartet.

These types are used by purescript-arrays. I'd like to Ramda do the same.

@buzzdecafe

This comment has been minimized.

Member

buzzdecafe commented Dec 9, 2014

we just moved all the types 😝

@buzzdecafe

This comment has been minimized.

Member

buzzdecafe commented Dec 9, 2014

ok, after mulling this over, i think i am 👎 on this. I'd rather compose with Maybe (or other Functors) than bake them in to primitive operations like head.

@davidchambers

This comment has been minimized.

Member

davidchambers commented Dec 9, 2014

How would that work with tail, for example?

@buzzdecafe

This comment has been minimized.

Member

buzzdecafe commented Dec 9, 2014

the contract of tail is already null-safe, `cuz it returns a functor, an array.

@davidchambers

This comment has been minimized.

Member

davidchambers commented Dec 9, 2014

It's safe, yes, but also lossy:

R.tail([1])  //=> []
R.tail([])   //=> []

Whereas with maybes these cases are distinct:

R.tail([1])  //=> Just([])
R.tail([])   //=> Nothing()
@buzzdecafe

This comment has been minimized.

Member

buzzdecafe commented Dec 9, 2014

i don't see why the proposal is an improvement over simply letting the user compose for themselves: R.compose(Maybe, R.head). tail is only trickier if you don't agree that the tail of an empty list is an empty list. That seems perfectly reasonable to me.

@davidchambers

This comment has been minimized.

Member

davidchambers commented Dec 9, 2014

Having Maybe treat null/undefined specially seems problematic. It makes the following expressions equivalent:

Maybe(R.head([undefined]))
Maybe(R.head([]))

These are not equivalent in my mind: I expect Just(undefined) in one case and Nothing() in the other.

@buzzdecafe

This comment has been minimized.

Member

buzzdecafe commented Dec 9, 2014

You can re-implement Maybe in terms of Just | Nothing (see #123) if you want, but I bet the case above will be difficult to resolve satisfactorily.

I expect Just(undefined) in one case and Nothing() in the other

That is because of weird-ass JS. Just(undefined) is pretty crazy.

@davidchambers

This comment has been minimized.

Member

davidchambers commented Dec 9, 2014

I don't see this as a JavaScript-specific issue. Take the tuple (None, None, None) in Python, for example. What's the head of this tuple? None. But were a Python implementation of Ramda to return None as the head of the empty tuple we'd have no way to differentiate the two cases.

@buzzdecafe

This comment has been minimized.

Member

buzzdecafe commented Dec 9, 2014

yes, that's the situation we are in: head([undefined, 1, 2, 3]) === head([]). This doesn't cry out to me for a solution.

Maybe(R.head([undefined])) == Maybe(R.head([])) (like Nothing, in the current impl): I also don't see this as much of a problem. If you are composing these things why would you want to pass undefined to the next function? That seems strange to me.

Aside: A quick look at https://github.com/folktale/data.maybe/blob/master/lib/maybe.js doesn't back me up here, allowing Just explicitly to wrap null or undefined values. On the other hand, @DrBoolean 's pointfree-fantasy (https://github.com/DrBoolean/pointfree-fantasy/blob/master/instances/maybe.js) interprets null and undefined as Nothing, much as our existing Maybe does. So this may be an open question.

And another thing: Why favor Maybe over other functors? What if I want an Either or a Validator or my own data type? That is another argument in favor of the compositional approach.

@CrossEye

This comment has been minimized.

Member

CrossEye commented Dec 10, 2014

I have several thoughts here.

  • Adding the algebraic types like Maybe when the user wants them sounds perfect for Ramda. I'm very glad that we have the extensions. I think they are still far from complete, but they're worthwhile. On the other hand, forcing them on all users of the library, whether they understand the notions or not seems very much against our practical mantra. Remember that although we might end up poorly implementing half of Haskell, doing so is not our driving goal.
  • Working with undefined is interesting. To my mind, we should never explicitly deal with undefined any differently than we deal with 'does not exist'. That the language has a few back doors, as with the arguments object, does not mean that we should treat them as API guidelines. So to my mind, R.head([undefined]) should be treated precisely the same as R.head([]). This is different from how one would deal with null. I think that treating null as an intentional blank value and undefined as the lack of a value is as close as we can get to the spirit of the language.
  • Now that work on #481 is back underway, we may end up with some sort of error-checking, but I would expect that to end up in a supplemental version, not in the main ramda.js. I'm simply wary of adding too much hand-holding to Ramda. I've thought of it all along as a library for those who know what they're doing. I'd like it to be beginner-friendly, but don't want to go far out of our way to blunt any sharp corners, especially at a cost of increased complexity.
@AutoSponge

This comment has been minimized.

Contributor

AutoSponge commented Dec 10, 2014

R.head([undefined]) should be treated precisely the same as R.head([])

I'm missing something... My understanding is that ES6 array methods will ignore "holes". If I pass undefined to a function, that should hardly be the same as passing no arguments.

@davidchambers

This comment has been minimized.

Member

davidchambers commented Dec 10, 2014

I think that treating null as an intentional blank value and undefined as the lack of a value is as close as we can get to the spirit of the language.

I avoid differentiating these values in code I write. I like to pretend there's just one nil value (which I call "null", but which others sometimes call "undefined"). I have my linter configured to allow == null checks. Just a day or two ago we added R.isNil which encourages us to think of both values as nil.

@buzzdecafe

This comment has been minimized.

Member

buzzdecafe commented Dec 10, 2014

If I pass undefined to a function, that should hardly be the same as passing no arguments.

i agree, but composing by passing down undefined feels pretty weird to me. that's where I'd be likely to interpose a functor.

@megawac

This comment has been minimized.

Contributor

megawac commented Dec 10, 2014

Another option would be to return R.__ if the value is out of bounds. That way the user could override it if they're particularly keen on handling this case

@megawac

This comment has been minimized.

Contributor

megawac commented Dec 10, 2014

Further, the Maybe implementation can be configured to respect that value as well

@CrossEye

This comment has been minimized.

Member

CrossEye commented Dec 10, 2014

@AutoSponge:

If I pass undefined to a function, that should hardly be the same as passing no arguments.

But, at least in the single-argument case, the only way to distinguish them is by querying the length of the arguments object. That hardly seems like a useful distinction.

var f = function(a) {return typeof a;}
f(); // "undefined"
f(undefined); "undefined"

@davidchambers:

There are definitely times when we want to conflate the two notions. But they do represent different concepts in JS. While null admits that we've heard about something but not bothered to assign it, undefined denies that such a property even exists. The fact that it's a value you can actually pass around strikes me more as an implementation accident than as fundamental API design.

So when we want to ask "Is it there?" it's fine to conflate the two. But for "Did we pass/define it?" I think they need to be distinguishable if we're to work with the grain of the language.

@CrossEye

This comment has been minimized.

Member

CrossEye commented Dec 10, 2014

Also, @AutoSponge:

My understanding is that ES6 array methods will ignore "holes".

Also, we're not particularly concerned with exactly what native implementations do nor with other languages or other libraries do. We look to them for inspiration, but not guidance.

@AutoSponge

This comment has been minimized.

Contributor

AutoSponge commented Dec 10, 2014

@CrossEye, Ramda currently treats undefined no differently than any other value, as evidenced by any iterator. So, as long as undefined appears in a collection, it is a value and as such exists. I'm just looking for clarity and consistency when treating sparse collections. Either undefined is a value or it isn't.

@CrossEye

This comment has been minimized.

Member

CrossEye commented Dec 10, 2014

@AutoSponge: and that's where I see little distinction between null and undefined. We've tried to make it clear that our basic collection type is a list. Since there is no true list implementation in JS, we live with arrays, but we don't even try to define the behavior for sparse arrays, since they don't represent lists.

@buzzdecafe

This comment has been minimized.

Member

buzzdecafe commented Dec 12, 2014

@davidchambers can this be closed, or do you wanna fight for it?

@davidchambers

This comment has been minimized.

Member

davidchambers commented Dec 12, 2014

I would like Ramda to encourage and enable clear, correct JavaScript code. With Ramda one can write more declarative code than is possible with vanilla JavaScript; Ramda undoubtedly encourages and enables clear JavaScript code. By removing the need for imperative loops and variable assignment Ramda also goes some way towards improving code correctness. I would like the library to go further in this direction. Cannot read property '_____' of undefined arrives in my inbox dozens of times per week. Many of these errors result from expressions such as list[0].foo (or the equally unsafe Ramda equivalents). These are, of course, preventable, though the burden lies solely on the programmer.

Taking another step down the path towards guaranteed correctness may violate Ramda's philosophy of being practical, though I do wonder whether practical in this case really means easy to use. One option I'm considering is testing these functions in the wild in a custom Ramda build. After a few weeks or months I could then report back on the experience.

@buzzdecafe

This comment has been minimized.

Member

buzzdecafe commented Dec 12, 2014

for the record, i'm not so concerned about the "practical" part of the slogan--I feel like that ill-defined term gets thrown out there to support anybody's hobbyhorse, and so i usually find that an unpersuasive argument.

but I am concerned about adding overhead to fundamental functions that can easily be composed in if desired.

(Incidentally, way back when, tail([]) //=> null! also see #20 (comment))

@davidchambers

This comment has been minimized.

Member

davidchambers commented Dec 12, 2014

I am concerned about adding overhead to fundamental functions that can easily be composed in if desired.

My view is that safety should be opt-out rather than opt-in.

If one knows list is non-empty, one can use R.toUpper(R.unsafeHead(list)).

@CrossEye

This comment has been minimized.

Member

CrossEye commented Dec 12, 2014

What this really makes me want to do is fold Maybe back into core.

But I'd still rather make this a step you can compose yourself. Those who don't want to deal with a Maybe shouldn't need to do so.

Regarding "practical", I've been working again on that Philosophy Of Ramda document, and using "a practical functional library for Javascript developers" as a framing mechanism. But I started from the back forwards, discussing what it means to be for developers, how it's focused on Javascript qua Javascript and not a poor-man's Haskell/Clojure/Scala/whatever, how it hangs together as a library, and have nearly finished the section on what it means to be functional. Now I have to write the "practical" section. I think I have some meaningful things to say there, but I agree that it's the most squishy term in our mantra.

@buzzdecafe

This comment has been minimized.

Member

buzzdecafe commented Dec 12, 2014

@CrossEye , will that article arrive before Perl 6?

@buzzdecafe

This comment has been minimized.

Member

buzzdecafe commented Dec 12, 2014

also, we could add maybe (little "m") as a function decorator, a la Allonge

this would mean checking on the way in, rather than the way out.

@DrBoolean

This comment has been minimized.

DrBoolean commented Dec 12, 2014

FWIW, I found that once a user enters the world of "container programming" - that is to say, monadic/functor based types, intimate familiarity with the "container apis" is essential.

var get = curry2(function(key, obj){ return Maybe(obj[key]); })
var head = get(0)

var garyBday = compose(map(add(1)), chain(get('age')), head)

var xs = [{name: "Gary", age: 30}, {name: "Lisa", age: null}]
garyBday(xs)

Here we have to know about chain to get things done. As soon as one starts using Either, Promise/Future/Stream, IO, etc, an understanding of traverse becomes essential. If those users were in a Future, we would want monad transformers. It seems to cascade and escalate fairly quickly, taking over the entire app (which is great if you want safety!)

I like using these things to enforce safety and purity, but I see the learning curve skyrocket. Perhaps a safety_net.js include could just overwrite some functions:

var head = compose(Maybe, _.head)

Just a thought. But, yeah, i think the adoption rate would decrease if enforced.

@CrossEye

This comment has been minimized.

Member

CrossEye commented Dec 12, 2014

If you're pushing for safety of this sort, do you suggest we try to capture the sort of things @AutoSponge was mentioning too?

// map(square) :: ?? not [Integer] -> [Integer], since 
map(square)([1, 2, , 4]); //=> [1, 4, NaN, 16]

// map(toUpper) :: ?? not [String] -> [String], since
map(toUpper)(["a", "b", , "d"]); //=> throws TypeError

So should map really have a different signature?

@AutoSponge

This comment has been minimized.

Contributor

AutoSponge commented Dec 12, 2014

In @CrossEye's example, above, fault can be laid with...

  1. the list should not produce/contain non-integers
  2. user's "pipe"; the list should have been filtered first
  3. toUpper should be null-safe
  4. map should skip empty/sparse values

Depending on who you blame for this TypeError will determine how it gets fixed. It's easy to push this to the user/coder however I seem to run into more and more APIs with unstructured data and I don't want to have to pre-process every response--but maybe that's the case for transducers being made again.

@DrBoolean

This comment has been minimized.

DrBoolean commented Dec 12, 2014

I'm not suggesting we enforce type safety in a dynamic language. Besides, there are other projects that could work with ramda to accomplish this.

I'm just thinking monadic programming should be optional and it'd be sweet to opt-in to the behavior somehow.

i'd hate to see the adoption rate decrease despite my obvious bias toward the haskell way

@CrossEye

This comment has been minimized.

Member

CrossEye commented Dec 12, 2014

@DrBoolean For "you" read "@davidchambers". 😄 Sorry about that.

I agree that there is a very sweet spot for monadic programming. I just don't think Ramda's core is it.

@CrossEye

This comment has been minimized.

Member

CrossEye commented Dec 12, 2014

@AutoSponge: I was really using this as an example of the sort of thing we've chosen not to deal with so far in Ramda, mostly for comparison's sake.

There are good questions to be had about sparse arrays. I just don't really want to ask or answer them, as they are not the type we are trying to model with Ramda, the list. Unfortunately, a JS array can have holes as well as arbitrary non-integer named properties; none of these really has anything to do with the only structure we care about for many of our functions.

megawac referenced this issue in jamiebuilds/backbone-errors Dec 13, 2014

@davidchambers

This comment has been minimized.

Member

davidchambers commented Jan 29, 2015

These list functions are available in Sanctuary, a JavaScript library for manipulating values without null checks. Sanctuary plays nicely with Ramda, and takes advantage of R.map's dynamic dispatch.

@buzzdecafe

This comment has been minimized.

Member

buzzdecafe commented Jan 29, 2015

Sanctuary looks pretty nice

@CrossEye

This comment has been minimized.

Member

CrossEye commented Jan 30, 2015

Very nice!

@buzzdecafe

This comment has been minimized.

Member

buzzdecafe commented May 25, 2016

Hey folks, just so I'm on the right track. If not using fantasyland types would this be a safealternative.

R.head = R.curry(function(a){
  let n = R.nth(0);
  return R.isNil(n) ? [] : n;
});

@austbot i intend a PR to add Just and Nothing to R, i just haven't had time. See also: #1749

Then you could just:

const safeHead = (xs) => xs.length === 0 ? Nothing() : Just(R.head(xs));

in your example: why [ ] for the head of the empty list? also, n will be a function, and you are not using a in the body of the function. Probably meant R.nth(0, a)? And if you have undefined values in your list that you want for some reason, the test may or may not be what you intend. IOW, you may be better off checking the length of the list rather than grab the element at index 0.

@austbot

This comment has been minimized.

austbot commented May 25, 2016

Saw that, removed it to not confuse people :)!

@buzzdecafe

This comment has been minimized.

Member

buzzdecafe commented May 25, 2016

i'm sorry you did, it makes my comment a non sequitur

@austbot

This comment has been minimized.

austbot commented May 25, 2016

Github backup please!!! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment