strange problem with detect method and cloning data

[buda]

I have a class

var a = Class.Create({

function initialize() {
var _items = [];

Object.defineProperties(this, {
'items': {
get: function () {
return Object.clone(_items);
},
enumerable: true
}
});
...
}
....
function indexOf(item) {
return -1;
}
....
});

then I create an instance of the class

var b = new a();

then I fill the _items with the data

[
{ name: 'Joe', last: 'Celko' },
{ name: 'Ivan', last: 'Susanin' }
]

and ovewrite the indexOf method with thw new one

b.indexOf = function(item) {
var index = -1;
var items = this.items;

items.detect(function(el, idx) {
var b = (el.name === item.name) && (el.last === item.last);
b && (index = idx);
return b;
});
return index;
};

then i try to seach

b.indexOf({ name: 'Joe', last: 'Celko' });

and always get -1 !!!
I try to debug and detect the this._each in Enumerable.each never goes
into, but if I chnge

return Object.clone(_items); --->> return _items;

everything goes ok and returns 0! Why?
Help me please. I cannot expose original items because changes on it
maybe damaged

[T.J. Crowder]

Hi,

The code you've quoted has a variety of syntax errors and typos, such
as using `function initialize()` within an object literal (which
results in an "Unexpected identifier" error) and using `Class.Create`
rather than `Class.create`. It's very hard to help when the code
presented is fundamentally broken and yet reported as working.

I recommend going back to first principles, building a minimal failing
test case, and if you don't solve the problem in the course of doing
that (which frequently happens), putting it on jsbin.com or
jsfiddle.net or similar and then linking to it so people can help you.
--
T.J. Crowder
Independent Software Engineer
tj / crowder software / com
www / crowder software / com

[T.J. Crowder]

On Oct 17, 10:17 am, "T.J. Crowder" <t...@crowdersoftware.com> wrote:
> It's very hard to help when the code
> presented is fundamentally broken and yet reported as working.

Sorry, in print that comes across much more harshly than it was
intended to. I only mean that it's easier to help debug code that's a
bit more cleaned up, wasn't meant as a dig.

-- T.J.

[buda]

here the code http://jsfiddle.net/QW8vM/10/

[T.J. Crowder]

On Oct 18, 6:01 am, buda <www...@pochta.ru> wrote:
> here the code http://jsfiddle.net/QW8vM/10/

`Object.clone` returns a raw object with a shallow copy of the
properties of the object you give it. It is not a perfect copy of the
object down to the prototype level:

var a, b;
a = [1, 2, 3];
b = Object.clone(a);
display("Object.isArray(a): " + Object.isArray(a)); // "true"
display("Object.isArray(b): " + Object.isArray(b)); // "false" <===

If you want to copy an array, use Array#slice (but note that it only
copies numeric properties, so if you've added non-numeric properties
to the array, you'll have to handle them):

var a, b;
a = [1, 2, 3];
b = a.slice(0); // Copy the array
display("Object.isArray(a): " + Object.isArray(a)); // "true"
display("a = " + a.join(", ")); // "1, 2, 3"
display("Object.isArray(b): " + Object.isArray(b)); // "true"
display("b = " + b.join(", ")); // "1, 2, 3"

Here's a working version of your fiddle:
http://jsfiddle.net/QW8vM/11/

Somewhat off-topic, but I find that `a` constructor very odd. `add`
always adds to a central array shared by all instances created by the
`a` constructor, but *every access* to the `items` property *copies*
that central array. Making a copy of an array on property access is
very surprising behavior, that's normally the sort of thing a function
is for. Consider:

var foo = new a();
var x = foo.items;
var y = foo.items;

Anyone reading that could would expect that `x` and `y` referred to
the same array, but of course they don't in the case of your `a`
constructor. Very surprising. Similarly:

var one = new a();
var two = new a();
one.add("foo");
one.add("bar");
alert(two.items.length); // "2"?!?! I haven't added anything to `two`!

I'm curious what the use-case is...

HTH,

[buda]

that sample was to demonstarte Object.clone bug :)

Usually I us to

var a  = Class.create((function() {

   var _privates = [];

   

   function initialize() {

      _privates.push({});

      this.internalId = _privates.length-1;

      _privates[this.internalId].items = [];  <-- here the instances items

      ...

[T.J. Crowder]

On Oct 18, 10:29 am, buda <www...@pochta.ru> wrote:
> that sample was to demonstarte Object.clone bug :)

It's not a bug, though I'd say detecting that it's being fed an array
wouldn't be a bad feature to add.

> Usually I us to
>
> var a  = Class.create((function() {
>
>    var _privates = [];
>
>    function initialize() {
>       _privates.push({});
>       this.internalId = _privates.length-1;
>       _privates[this.internalId].items = [];  <-- here the instances items
>       ...

How do you ever clean up the private data for instances that have been
released and reclaimed by the GC? Some kind of destroy contract and
hope you don't miss out a destroy call?

-- T.J.

[buda]

I use destroy method on the class and clean everythig in it

[buda]

T.J. here is the example of using instance private vaiables store in use - http://jsfiddle.net/QW8vM/17/

[T.J. Crowder]

Hi,

On Oct 20, 3:33 am, buda <www...@pochta.ru> wrote:
> T.J. here is the example of using instance private vaiables store in use -http://jsfiddle.net/QW8vM/17/

FWIW, that code refers to an undefined symbol `_items` in the property
getter function. Also note that it has a memory leak: `destroy`
releases the `items` on the private variables object you create for
every instance, but doesn't actually release the private variables
object itself or the array entry associated with it. So every instance
creation/destruction will leak a small amount of memory and as time
goes by, the array will get more and more bogged down in abandoned
entries.

Here's a version without those issues (note that `_privates` is now
just an object, not an array, and see also changes in `destroy`):
http://jsbin.com/ewoniq

Unless you're going to have lots and lots of these objects in memory
at the same time (like, thousands), I'd *strongly recommend* you avoid
this pattern. Instead, just define `add` and `indexOf` within
`initialize` and have them use `_items` (the local var in
`initialize`) directly. This increases the overhead per instance
(because each and every instance gets its own `add` and `indexOf`
functions), but avoids requiring the `destroy` call. Example:
http://jsbin.com/ewoniq/2

FWIW,

[buda]

Hi!

Thanks for response.

 `_items` - it's typo ;) of course there mightbe - return _privates[this.internalId].items.slice(0);

I agree with you about error in destroy - not deleting an instance item in _privates array. 

About not using such pattern - I agree if only few methods need to access _privates, but what if an object has about 10 or more methods that manipulate _privates!? make them all per instance?

[T.J. Crowder]

On Oct 21, 2:03 pm, buda <www...@pochta.ru> wrote:
> About not using such pattern - I agree if only few methods need to access
> _privates, but what if an object has about 10 or more methods that
> manipulate _privates!? make them all per instance?

It depends entirely on how many instances you expect to have. If
thousands, enable function reuse (I just tend to use a naming
convention rather than true private variables); if dozens, probably
just go ahead and duplicate the methods.

[buda]

Hi T.J.!

Thanks for your help!

This code will be sheared and there is the risk that people have an ability involuntarily to change privates - so I need to protect it