2

This sort of thing works in JavaScript

function main() {
    return 1;
}
main.sub = function () {
    return 2;
};

main();     // 1
main.sub(); // 2

and seems useful for doing stuff like

function props() {
    return { color: props.color(), size: props.size() };
}
props.color = function () {
    // calculate and return color
};
props.size = function () {
    // calculate and return size
};

so that you'd have an easy way to pull in an object of all the props using prop() but if you only need one you can call for it directly. Is that type of setup okay?

ryanve
  • 50,076
  • 30
  • 102
  • 137
  • 1
    Interesting. I didn't do this before. Personally, I think it's pretty cool :-) – Sergio Tulentsev Jan 18 '12 at 07:41
  • @SergioTulentsev I was a little surprised at first, but it makes sense because in JavaScript everything is an object (except for primitive values like booleans and numbers). Related: https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/Function and http://mirkokiefer.com/blog/2010/02/everything-is-object-in-javascript/ – ryanve Jan 18 '12 at 07:54

3 Answers3

2

Even though it is absolutely legal, I'd say it is the wrong utilization of sub methods. It confuses the function with the return value of said function.

I would say a proper use of submethods within functions is when you want to add metadata for the function. Let's say you want to set a property for the function like documentation or whether you want it to be obfuscated. Then you can set the property for the function instead of the underlying object.

Even though your usage may save some writing, it makes reading the code much harder IMHO. You should always strive for ease of readability, not of writing.

Luis
  • 1,294
  • 7
  • 9
  • Good points. So then I take it you'd rather have something like `props.all()` for getting all of em'. – ryanve Jan 18 '12 at 07:57
  • I think a full answer to this can be found here: http://stackoverflow.com/questions/208016/how-to-list-the-properties-of-a-javascript-object – Luis Jan 18 '12 at 08:00
  • 1
    or implement it as a closure. var props = function() { var theColor = function() {}; var theSize = function() {}; return { color: theColor(), size: theSize() }; This is a better pattern in general as it promotes encapsulation – Sam Giles Jan 18 '12 at 08:01
  • @Luis That's not related the poster is asking whether this is an "OK" idiom to use. – Sam Giles Jan 18 '12 at 08:03
  • @SamGiles I thought long and hard on the best way to phrase the question and 'okay' seemed the best fit LOL – ryanve Jan 18 '12 at 08:26
  • @SamGiles I was answering just to how to get all properties. I agree it is not completely related to whether it is OK. – Luis Jan 18 '12 at 19:29
1

That looks useful, but it isn't very obvious what's happening when you use it.

The expression props.color() returns the same as the similar expression props().color, but the performance differs as the latter also calculates the other properties, which are then discarded. It's easy to misuse the feature without noticing, so you should consider using an approach where the usage shows more clearly what's actually happening in the code.

Guffa
  • 687,336
  • 108
  • 737
  • 1,005
  • Exactly. Performance was partially what led me to this. If you need all (or in the case) both of the props for use in a function, you could create a local `var props = props();` and then access `props.color` etc. in the local scope. If you only need one of them then you'd use `props.color()` and not have to calculate all of them. – ryanve Jan 18 '12 at 08:02
1

Generally this is probably not good practice:

calling: props.color(); will do the same thing as calling props().color.

What would be a better pattern would be something as follows:

var props = function() {
 var theColor = function() {
  // calculate the color
 };

 var theSize = function() {
  // calculate the size
 };

 return {
  color: theColor(),
  size: theSize()
 }
}

var someprops = new props();

You could instead of having for example theColor() as the object for color, you could leave it as the function: So the return would be

return {
 color: theColor,
 size: theSize
}

The difference being that the props.color == "function" whereas in the previous example props.color would've equaled the result of the function.

Sam Giles
  • 650
  • 6
  • 16
  • I like that in general but for this I think being able to access `props.color()` is a benefit, because `props().color` causes you to run thru all the calculations if you only need that one. – ryanve Jan 18 '12 at 08:20
  • Yes, however in the final example, if the return type is simply the function, no processing occurs until the function is called: `var someprops = new props(); var something = someprops.color();` You're only calling the function as you need it then, remember even functions are objects you can pass around. – Sam Giles Jan 18 '12 at 08:27
  • That's it! =) The 2nd way, without `()` in the return. It's the same speedwise but takes less code. See http://jsperf.com/function-object-vs-closure and http://jsperf.com/closure-methods-performance Plus, I *think* `new` is only needed if referring to the object using `this` in the methods. If you just need one of them you can even just do use `props().color()`. See http://jsperf.com/closure-methods-performance-with-new – ryanve Jan 18 '12 at 16:16
  • Try minifying both versions from http://jsperf.com/closure-methods-performance with http://closure-compiler.appspot.com/ It minifies them both the same way. – ryanve Jan 18 '12 at 16:35
  • You are correct about using the `new` keyword however, I tend to always use new: see http://www.jslint.com/lint.html#new for reasons. It depends on what context you are using `this` in really. – Sam Giles Jan 19 '12 at 09:50