Map shouldn't accept a value. Move that to value()

[buchanae]

I can see the idea behind allowing map to accept a value, and it's probably useful, but it's also confusing and breaks a long standing contract (the signature of a map function). The reason I'm here is because of this:

class Index {
  lookupValue { ...pretend code... }

  getValueAtTime(time) {
    return this.lookupValue(time);
  }
}

var index = new Index();
var stream = someOtherStream.map(index.getValueForTime);

This returns a stream of undefined.
How long did it take you to see the error there? Took me hours.

Highland.value(val) (there's maybe a better name) would clear this up and save me some sanity.

[svozza]

I didn't even realise this was a feature of our map. Big +1 on this.

[vqvu]

I agree that it's confusing. I don't even see it being all that useful (as evidenced by the fact that I didn't even know this behavior existed!). Unfortunately, it's documented, and we can't break backwards compatibility in 2.x.

Highland contributors, what are your thoughts on changing this behavior in 3.0? map is ubiquitous enough that nobody would bother reading the docs for it (imo) or expect that it can take non-functions as arguments. Splitting the behavior into a separate value/constantMap seems better to me.

[LewisJEllis]

Definite +1 for 3.0.

[jeromew]

I started using it last week after seeing it in the doc so I suppose that other people use it to and it is indeed a breaking change.

+1 though for 3.x. We have some duck typing also in the constructor but I agree that for map it can be a debugging hell.

Do we have a solution for deprecation warning ?

[svozza]

I guess a name like mapK might make sense because this is really just sugar for a partially applied K combinator.

const K = _.curry((x, y) => x);
_([1, 2, 3]).map(K('hi')) // => 'hi', 'hi', 'hi'

Although tbh, I'm not sure it warrants a function in the lib given how easy it is roll your own.

[vqvu]

Do we have a solution for deprecation warning ?

Besides updating the docs to say so? I don't think so.

I'm not sure it warrants a function in the lib given how easy it is roll your own.

Having a _.constant = K might make sense to supplement _.compose, _.curry and _.partial. For completeness.

I will add an entry in our grand list of 3.0 issues (#179).

[apaleslimghost]

Having a _.constant = K might make sense to supplement _.compose, _.curry and _.partial. For completeness.

This is another instance I think we should be moving towards less functions. We already have things like pluck which are just map plus some combinator. People can roll their own, or use Ramda (or lodash-fp, or prelude.ls, or [insert combinator library here]). I'd be fine with having a highland-kitchen-sink module once The Great Modularisation happens.

Edit:

Do we have a solution for deprecation warning ?

Node has util.deprecate, which warns when a function is used (and can be turned off by command line flags). It also works in browserify.

[svozza]

Hehe. I was wondering when we'd see you, @quarterto , I remember the trouble I got into when I tried to add the I combinator.

[svozza]

Fixed by #408