0

I have this Class/Object:

function Shape(){
    this.xArray = new Array();
    this.yArray = new Array();
    this.selectedColor;
}

In my code I keep creating objects and adding them to another array. Like this:

var shapes = new Array();
var current = new Shape();
function onmousemove(e){
    current = new Shape(); // won't create a new ojbect!!
    current.xArray.push(e.pageX, e.pageY);
    ...... 
    shapes.push(current);
}

The problem is that I keep getting the same object, even when I create a new shape. I'm new in javascript, am not sure if am doing this right.

UPDATE: here is my code without editing:

function handleMouseDown(e)
{
currentshape = new Shape(); 
showMessage(currentShape.xArray.length);
currentShape.xArray.push(e.clientX- this.offsetLeft);       
currentShape.yArray.push(e.clientY- this.offsetTop);
shapes.push(currentShape);
draw();
mouseDownFlag = true;
}

function handleMouseMove(e)
{   
if(mouseDownFlag)
{
    currentShape.xArray.push(e.clientX- this.offsetLeft);
    currentShape.yArray.push(e.clientY- this.offsetTop);

    draw();
}
}

the showMessage function just alters a div to show me result I keep getting a cumulative number of the xArray from all shapes.

Improve this question
3
  • 2
    That's not possible. the new keyword results a new instance of Shape. To verify, add alert(current === new Shape) to your code (anywhere). Commented Jan 5, 2012 at 11:57
  • Can you show the code that proves you are getting the same object? Nothing in what you've posted already ever uses the shapes array. Maybe you could put a demo on jsfiddle.net. Also, if you're creating (or trying to create) a new Shape every time the mouse moves why have an array for the x and y coordinates? You don't get to store the next move in the same arrays because you (try to) create another Shape. Commented Jan 5, 2012 at 12:12
  • It is up there, the "showMessage(currentShape.xArray.length);" line. every time I click, I need to start a new shape, so I should get a zero length xArray. but this line shows me that am getting an addition to the previous shape. Commented Jan 5, 2012 at 12:16

3 Answers 3

Reset to default
3

Update

The bug is this line in handleMouseDown:

currentshape = new Shape();

You've used currentshape, not currentShape (with the capital S). That creates a separate and unrelated implicit global rather than updating the currentShape one, since JavaScript is case sensitive (s and S are two different variables).

To avoid this sort of problem in the future, you might consider using the strict mode offered by the latest version of JavaScript, ECMAScript 5th edition. In strict mode, amongst other things, you no longer have the horror of implicit globals. Assigning to an unresolvable reference causes an error instead.

To do that, you'd use a scoping function around all of your code, and put "use strict"; at the top of it, like this:

(function() {
    "use strict";

    var currentShape = new Shape();

    function handleMouseDown() {
        // ...
    }

    function handleMouseMove() {
        // ...
    }

    // ...

})();

This is totally backward-compatible with JavaScript engines that don't support the new strict mode yet, so it doesn't cause trouble on older browsers (you just don't get the benefits of strict mode).

This also has the advantage that you're not introducing any global variables into an already-crowded global variable space. currentShape is shared by the functions inside your scoping function, but neither it nor handleMouseDown or handleMouseMove is global.

One side-effect of doing that is that functions referenced from event handlers created via onXYZ attributes in your HTML markup need to be global, and so if none of your functions is global, you can't use them anymore; you use addEventListener / attachEvent to hook up event handlers instead. That's usually a good idea anyway, but if you absolutely must use onXYZ handlers, you can do so by making selected functions globals by assigning them to properties on window, e.g. window.handleMouseMove = handleMouseMove; (live example). In general, though, best to avoid any globals at all and use the addEventListener / attachEvent (aka "DOM2" style) of event handlers.

Old answer before you posted your real code:

If the code really is as quoted, you'll create a new Shape object every time onmousemove is called.

I suspect you may have removed the code that was actually causing the problem when simplifying it to add it to your question. For instance:

var shapes = new Array();
var current = new Shape();
var foo = current;            // <<==== New code
function onmousemove(e){
    current = new Shape();
    current.xArray.push(e.pageX, e.pageY);
    ...... 
    shapes.push(current);
}

With that code, although current will be set to a new Shape on every call to onmousemove, foo will always point to the initial Shape, because there is no enduring connection between the variable foo and the variable current; current can get a new value without affecting the value held by foo.

So if your real code has something like that (passing current into a function, assigning it to another variable, or any of several other things), that would be the problem.

Improve this answer
Sign up to request clarification or add additional context in comments.

4 Comments

@user836252: You've left off the code where you're creating the global currentShape and such -- exactly the part of the code I pointed to above as being the possible problem.
@user836252: Never mind, found the bug, it's a simple typo. See updated answer.
OMG! I spent 4 hours looking for it. what can i say? but thanks lol
@user836252: :-) Glad that helped. I've updated the answer with something to help you avoid wasting those four hours in the future. Happy coding!
3

forgive me but... why that approach ?

IMHO it's better to have something like

function Shape(xx, yy, sc) {
 this.x = xx;
 this.y = yy;
 this.selectedColor = sc;
}

var shapes = [];

function onmousemove(e) {
  var cur = new Shape(e.pageX, e.pageY, "???");
  shapes.push(cur);
}

However in your code (using your Shape object) I think there's a little problem (and at least one unneeded Array) :

   var shapes= new Shape(); 

    function onmousemove(e){
        shapes.xArray.push(e.pageX); // Push the X
        shapes.yArray.push(e.pageY); // push the Y
    }

So you'll have just a shapes array, wich holds 2 arrays (one for the Xs and one for the Ys)

Improve this answer

2 Comments

By the used names, I deduce that xArray and yArray has to be a list. These properties probably describe the coordinates of the angles.
If OP wants to store an object containing an array of coordinates I think it's better to keep the coordinates together in an object, however that may lead to changes in other code, so I've edited my answer (hoping to have understood the question properly)
1

current is a global variable. just try to create a stack variable inside the function.

function onmousemove(e){
   var temp = new Shape(); // won't create a new ojbect!!
    temp.xArray.push(e.pageX, e.pageY);
    ...... 
    shapes.push(temp);
}
Improve this answer

3 Comments

@user836252: You need it to be shared by multiple functions. That doesn't mean it has to be global. :-)
thanks, but can you explain more? how can I share it by multiple functions?
you can pass the variable as a parameter to all the functions that you want to share the variable with..

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.