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

feature-methods #55

Merged
merged 2 commits into from
May 15, 2014
Merged

feature-methods #55

merged 2 commits into from
May 15, 2014

Conversation

jmdobry
Copy link
Member

@jmdobry jmdobry commented May 15, 2014

No description provided.

@jmdobry jmdobry self-assigned this May 15, 2014
@jmdobry jmdobry added this to the 0.9.0 milestone May 15, 2014
@jmdobry
Copy link
Member Author

jmdobry commented May 15, 2014

@kentcdodds What do you think?

@kentcdodds
Copy link
Contributor

Without having the context of the whole project, the stuff you changed looks good to me. I wonder if there's any way you can name the factory function the title case version of the resource name. The reason for this:

function User() {};
var user = new User();
console.log(user); // <-- User {}      (helpful)

var def = {};
def.factory = function() {};
var user2 = new def.factory();
console.log(user2); // <-- def.factory {}      (less helpful)

I'm not sure how to do this. I tried adding def.factory.name = 'User'; but that didn't take. It's not a huge deal, but it would be nice to have the resource name printed out when we're logging a resource.

@jmdobry
Copy link
Member Author

jmdobry commented May 15, 2014

I wanted to do that very same thing, but I couldn't find anything on how to
dynamically name a function expression.

@jmdobry
Copy link
Member Author

jmdobry commented May 15, 2014

@kentcdodds It looks like the only way to dynamically name a function is to use eval. In this case it might be okay because I have complete control over what would go into the eval.

@kentcdodds
Copy link
Contributor

If we can just get over our initial disgust of typing the word eval in code then we may just be able to do it ;)

jmdobry added a commit that referenced this pull request May 15, 2014
@jmdobry jmdobry merged commit fc09d51 into master May 15, 2014
@jmdobry jmdobry deleted the feature-methods branch May 15, 2014 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants