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

Add S.length function #159

Closed
wants to merge 1 commit into from
Closed

Conversation

svozza
Copy link
Member

@svozza svozza commented Feb 29, 2016

I went with the Countable suggestion as mentioned in #145.

@davidchambers
Copy link
Member

What's the appeal of this function over R.prop('length')? I'm curious. :)

@svozza
Copy link
Member Author

svozza commented Mar 1, 2016

Hmmm, good question. Not a lot, aside from a more descriptive error message than:

TypeError: Cannot read property 'length' of undefined

@davidchambers
Copy link
Member

I think we should start by defining a function for accessing record fields. It would be similar to R.prop but would throw if the requested property is not present.

@svozza
Copy link
Member Author

svozza commented Mar 3, 2016

How would that work? We couldn't do something like:

def('prop', {}, [$.RecordType, $.Any]...)

@davidchambers
Copy link
Member

How about something like this?

var prop =
def('prop',
    {a: [Accessible]},
    [$.String, a, b],
    function(name, obj) {
      if (name in obj) {
        return obj[name];
      } else {
        throw new TypeError('‘prop’ expected object to have a property named ' +
                            '‘' + name + '’; ' + show(obj) + ' does not');
      }
    });

@svozza
Copy link
Member Author

svozza commented Mar 4, 2016

Do we want to traverse the prototype chain to find properties? I know not doing so would make this an O(n) function but I think it might be less surprising to users.

@svozza svozza mentioned this pull request Mar 4, 2016
@davidchambers
Copy link
Member

How do you feel about length now that we have prop, Stefano?

@svozza
Copy link
Member Author

svozza commented Apr 19, 2016

Only seeing your comment now, David. I think we can close this one. I really don't like this Countable type thing.

@davidchambers
Copy link
Member

Okay. Closing without prejudice.

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

Successfully merging this pull request may close these issues.

2 participants