Subj : Re: Spidermonkey:  Need a 2nd set of eyes to find what I did wrong
To   : Izman <cburdon@tampabay.rr.com>
From : Brendan Eich <brendan@meer.net>
Date : Mon Sep 15 2003 05:10 pm

Izman wrote:

>When running a script that accesses the properties:
>
>        The value returned from JSGetProperty is 0 when accessing prop1 (as expected).  (i.e. "var a = myobj.prop_one;", a ends up with the value 0)
>        The value returned from JSGetProperty is "prop_two" when accessing prop2 (should return 1 as per the enum and property definitions).   (i.e. "var b = myobj.prop_two;", b ends up with the value "prop_two")
>  
>

What version of the engine are you using?

>I am missing something obvious I think, I just need some more eyes to look at it.
>
>Here is the code for the class:
>  
>

This seems based directly on the docs at 
http://sourceforge.net/docman/display_doc.php?docid=10853&group_id=50913 
as you say.  Cc'ing Franky at the last address I have for him, so he can 
make corrections per below.

>class JSMyObject
>{
>public:
> JSMyObject() {
>  m_CMyObject = new CMyObject();
> }
>
> ~JSMyObject() {
>  delete m_CMyObject;
>  m_CMyObject = NULL;
> }
>
> static JSClass MyObjectClass;
>
> static JSBool JSConstructor(JSContext *cx, JSObject *obj, uintN argc,
>jsval *argv, jsval *rval);
> static void JSDestructor(JSContext *cx, JSObject *obj);
>
> static JSBool JSGetProperty(JSContext *cx, JSObject *obj, jsval id, jsval
>*vp);
> //static JSBool JSSetProperty(JSContext *cx, JSObject *obj, jsval id, jsval
>*vp);
>
> static JSObject *JSInit(JSContext *cx, JSObject *obj, JSObject *proto);
>
> static JSPropertySpec MyObject_properties[];
> static JSFunctionSpec MyObject_methods[];
>
> enum {
>  prop1,
>  prop2
> };
>  
>

Giving tinyids negative values, starting from -1 and decreasing, is 
traditional, and allows the object to handle non-negative integer 
property ids (AKA array indexes) for different properties than these 
tinyid-named ones defined via the JSPropertySpec array.

>protected:
> CMyObject *getCMyObject() {
>  return m_CMyObject;
> }
>
>private:
> CMyObject *m_CMyObject;
>};
>
>JSObject *MyObject::JSInit(JSContext *cx, JSObject *obj, JSObject *proto)
>{
> JSObject *newObj = JS_InitClass(cx, obj, proto, &MyObjectClass,
>     JSMyObject::JSConstructor, 0,
>    NULL, JSMyObject::MyObject_methods,
>    NULL, NULL);
> JS_DefineProperties(cx, newObj, JSMyObject::MyObject_properties);
>  
>

This is silly, the JSMyObject::MyObject_properties array should be 
passed to JS_InitClass instead of the first NULL actual parameter, for 
the |ps| formal parameter.  That will define the properties, just as if 
you called JS_DefineProperties on the return value (the prototype object 
for the class).  There is no need for callers to do this work.

> return newObj;
>}
>
>JSBool JSMyObject::JSConstructor(JSContext *cx, JSObject *obj, uintN argc,
>                                     jsval *argv, jsval *rval)
>{
> JSMyObject *p = new JSMyObject();
> JS_SetPrivate(cx, obj, p);
>  
>

This code should check for errors.

More important, it must set *rval = OBJECT_TO_JSVAL(obj); in order that 
'myobj = new MyObject' to work correctly).

> return JS_TRUE;
>}
>
>void JSMyObject::JSDestructor(JSContext *cx, JSObject *obj)
>{
> JSMyObject *p = (JSMyObject *)JS_GetPrivate(cx, obj);
> delete p;
> p = NULL;
>  
>

There's on point in nulling p, as it is about to go out of scope.

I'm not sure what else is wrong, but please correct the above and let us 
know how things go.

/be

.