What is wrong with my javascript object?

Question

I have just started learning OOP in javascript and I am trying to re-write a simple program using OOP that I had previously written as a procedural program. The program is a reaction tester in which a random shape appears on the screen and the user must click it as quickly as possible. After clicking the shape, the time in seconds that it took for the user to respond is displayed.

I have not gotten very far, I am just trying to make a square of random size and random color appear on the screen, but I can't even manage that. See my code below:

<script type="text/javascript">

function Shape () {
  this.x = Math.floor(Math.random()*850);
  this.y = Math.floor(Math.random()*850);
  this.draw();
}

Shape.prototype.draw = function() {
  var shapeHtml = '<div></div>';
  var widthAndHeight = Math.floor(Math.random()*400);
  var left = Math.floor(Math.random()*850);
  var top = Math.floor(Math.random()*850);
  this.shapeElement = $(shapeHtml);
  this.shapeElement.css({
    position: "relative",
    left: this.left,
    top: this.top,
    width: widthAndHeight,
    height: widthAndHeight,
  });
  $("body").append(this.shapeElement);
}

Shape.prototype.colour = function() {
  var colours = '0123456789ABCDEF'.split('');
  var randomColour = "#";
  for (i = 0; i < 6; i++) {
    randomColour+=colours[Math.floor(Math.random()*16)];
  };
  this.shapeElement.css({backgroundColor: 'randomColour'});
}

var square = new Shape();


</script

So far, no square will appear on the screen. All that happens is a div of a random size is appended, but it is always in the upper-left position and has no background color. The console is not helping me because it is not showing that there are any errors in my code. I am extremely confused and finding the transition to OOP is extremely confusing. Any help in understanding why this won't work would be extremely appreciated!

If you want to "draw", the canvas element is pretty neat: developer.mozilla.org/en-US/docs/Web/API/Canvas_API/Tutorial — Jonathan Rys
– Jonathan Rys, Commented May 23, 2018 at 0:05
position: 'absolute' and backgroundColor: randomColor is what you want. You should probably call colour() somewhere too. Voting to close as a typo — Phil
– Phil, Commented May 23, 2018 at 0:14
Sorry, that should be backgroundColor: randomColour, the point being that you should use the variable randomColour and not a string literal 'randomColour' — Phil
– Phil, Commented May 23, 2018 at 0:34
I changed my code to reflect Phil's last comment, and it made absolutely no difference. — Michael Alexander
– Michael Alexander, Commented May 23, 2018 at 0:39

traktor · Accepted Answer · 2018-05-23 01:39:34Z

3

Several small errors:

Warning: function Shape sets up x and y properties that are not used.

Error: Shape.prototype.draw defines variables left and top but refers to them as this.left and this.top in the CSS object initializer. As properties they are undefined - take out the two this. qualifiers.

Error: Shape.prototype.colour is not called, so the DIV elements are transparent. Insert a call this.colour() after, say, setting the CSS.

Error: The css initialiser object value for background color should be the variable name, randomColour not the string literal 'randomColour'. Remove the quote marks from around the identifier.

Severe warning: the for loop in the colour function does not declare i and creates it as an implicit global variable. Insert "use strict"; at the beginning of script files or function bodies to generate an error for undeclared variables.

In summary none of the errors generate errors on the console (undefined CSS values are ignored) but work to prevent the code working.

edited May 23, 2018 at 1:39

answered May 23, 2018 at 0:50

traktor

19.7k5 gold badges37 silver badges56 bronze badges

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

2 Comments

Phil Over a year ago

position: relative probably isn't what OP wants either

traktor Over a year ago

@Phil for the case of appending a single DIV to an otherwise empty page it changes little. I agree that for presenting multiple DIV elements in random positions, absolute positioning is likely to be better.

J K · Accepted Answer · 2018-05-23 01:51:16Z

1

There are number of issues.

1) colour() method is never called.

2) Referring this.top and this.left inside the css construct won't work either.

3) randomColour is a variable, not a string literal.

Fixed the issues and embedded the code here. Have a look.

function Shape () {
  this.x = Math.floor(Math.random()*850);
  this.y = Math.floor(Math.random()*850);
}

Shape.prototype.draw = function() {
  var shapeHtml = '<div></div>';
  var widthAndHeight = Math.floor(Math.random()*400);
  var left = Math.floor(Math.random()*850);
  var top = Math.floor(Math.random()*850);
  this.shapeElement = $(shapeHtml);
  this.shapeElement.css({
    'margin-left': left,
    'margin-top': top,
    'width': widthAndHeight,
    'height': widthAndHeight,
  });
  $("body").append(this.shapeElement);
}

Shape.prototype.colour = function() {
  var colours = '0123456789ABCDEF'.split('');
  var randomColour = "#";
  for (i = 0; i < 6; i++) {
    randomColour+=colours[Math.floor(Math.random()*16)];
  };
  this.shapeElement.css({backgroundColor: randomColour});
}


$(document).ready(function() {
var square = new Shape();
square.draw();
square.colour();
});

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <title>Shape</title>
</head>
<body>
<div></div>
</body>
</html>

edited May 23, 2018 at 1:51

answered May 23, 2018 at 1:03

J K

9548 silver badges13 bronze badges

2 Comments

Michael Alexander Over a year ago

Thank you, this did help me understand much better. However, the random square still remains left-aligned and I can't seem to get it to appear at random coordinates in the page, and I have tried a few different ways. Do you know why this is?

J K Over a year ago

The css property should instead be margin-left and margin-top. Changed the script accordingly.

Collectives™ on Stack Overflow

What is wrong with my javascript object?

2 Answers 2

2 Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

2 Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related