Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #11481 (reopened defect)

Opened 2 months ago

Last modified 3 weeks ago

[PATCH] [TEST] Constructor wrapper should return value

Reported by: cch1 Assigned to: sam
Priority: normal Milestone: 2.x
Component: Prototype Version: edge
Severity: normal Keywords: TRIVIAL constructor initialize return
Cc:

Description

JavaScript allows constructor functions to return values, "and if it does so, that returned object becomes the value of the new expression."

[Quote from page 151 of JavaScript, the Definitive Guide, Fifth edition, aka the Rhino book]

Prototype class functionality extends the constructor to call this.initialize, but it ignores its return value. This makes it impossible to short circuit the new method and return cached objects, for example.

The attached patch to klass() simply passes the return value (if any) from initialize on to the caller and thus restores the standard JavaScript functionality.

Attachments

constructor_explicit_return.diff (1.5 kB) - added by cch1 on 03/31/08 16:36:48.
Removed debugging code from test.

Change History

03/31/08 14:07:11 changed by DerGuteMoritz

+1 but it'd better be tested! :)

03/31/08 14:08:23 changed by cch1

  • cc deleted.
  • keywords changed from constructor initialize return to TRIVIAL constructor initialize return.

03/31/08 14:10:43 changed by jdalton

I second this. Just the other day I was wondering why there was no return functionality.

Can you a patch for the base.html unit test as well. To confirm nothing goes wrong (I really don't think anything should)

03/31/08 14:11:04 changed by jdalton

  • summary changed from Prototype's constructor wrapper ignores return values from initialize to [PATCH] Prototype's constructor wrapper ignores return values from initialize.

03/31/08 14:40:05 changed by RQuadling

For what it is worth, +1.

But be aware that the you can not return scalars (boolean, int, string), only objects, things that can be created with new.

03/31/08 16:36:48 changed by cch1

  • attachment constructor_explicit_return.diff added.

Removed debugging code from test.

04/16/08 20:05:32 changed by kangax

  • summary changed from [PATCH] Prototype's constructor wrapper ignores return values from initialize to [PATCH] [TEST] Prototype's constructor wrapper ignores return values from initialize.

04/16/08 20:06:30 changed by kangax

  • summary changed from [PATCH] [TEST] Prototype's constructor wrapper ignores return values from initialize to [PATCH] [TEST] Constructor wrapper should return value.

04/16/08 20:34:32 changed by kangax

Here's a simpler test (can't make a proper diff at the moment)

var Dummy = Class.create({
  initialize: function() {
    return {foo: 'bar'};
  }
})

var dummy = new Dummy();
this.assertIdentical('bar', dummy.foo);

(follow-up: ↓ 11 ) 04/28/08 05:37:44 changed by jdalton

  • status changed from new to closed.
  • resolution set to wontfix.

After some discussions I don't believe this will ever make it into the core.

04/28/08 14:53:21 changed by cch1

Why?

(in reply to: ↑ 9 ) 04/28/08 14:53:57 changed by cch1

Replying to jdalton:

After some discussions I don't believe this will ever make it into the core.

Why?

04/28/08 15:00:26 changed by jdalton

var Dummy = Class.create({
  initialize: function() {
    return {foo: 'bar'};
  },
  method: function(){
    return 'method';
  }
})

var dummy = new Dummy();

dummy.foo; // => 'bar'
dummy.method; // => undefined
dummy instanceof Dummy; // => false

dumy isnt instanceof Dummy

04/28/08 15:16:51 changed by cch1

Your observations are accurate but don't answer the question of "Why".

  1. The JavaScript standard allows returning alternative objects from constructors.
  2. The JavaScript standard has the exact same restrictions you have noted.

So why has Prototype added this restriction?

I don't mean to be snotty, but observing that core JS itself is less than "regular" with this feature doesn't really answer the question of why Prototype has removed it. Is it Prototype policy to suppress functionality in core JS that is "irregular"?

04/28/08 15:19:24 changed by jdalton

You are free to mofiy your version of Prototype. Sam does not like the idea of dummy not an instance of dummy.

04/28/08 16:57:43 changed by jdalton

  • status changed from closed to reopened.
  • resolution deleted.

For the record I personally don't see the harm in allowing users to return values from the constructor. I will reopen ticket in hopes of future discussion.

04/28/08 17:41:10 changed by cch1

Thanks. FWIW, I did got a lot of traction in the #prototype forum when I first mentioned this patch. Far from being turned off by the irregularity of non-traditional returns from constructors, many wanted to be able to return 'false'. They lost interest when the learned that JS requires that the return be a descendant of Object (no primitive types allowed).

The biggest opportunity opened up by this patch is the return of cached values. If I have

var ExpensiveToFetchServerClass = Class.create({
  initialize: function(id) {
    var cached_value = ExpensiveToFetchServerClass.cache.get(id);
    return cached_value if cached_value;
    ...
    ExpensiveToFetchServerClass.cache.set(id, this);
  }
})

var x = new ExpensiveToFetchServerClass(25);  // Expensive fetch
...
var y = new ExpensiveToFetchServerClass(25);  // Return from cache.

And another more abstract but complete example:

var Factorial(n) = Class.create({
  initialize: function(n) {
    var cached_value = Factorial.cache.get(n);
    return cached_value if cached_value;
    var value = n * Factorial(n-1)
    Factorial.cache.set(n, value);
    return value;
  }
})

Anyway, thanks for keeping the door open.

-Chris