HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Single Rotate Method

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
rotatesinglemethod

Problem

I just started coding a simulation-like application. It's about our solar system. Right now, I'm working on rotating the moon. I have this constructor

function Mass(elm) {
    if (typeof elm == "string") {
        this.elm = document.getElementById(elm);
    } else {
        this.elm = elm;
    }
    this.width      = parseInt(window.getComputedStyle(this.elm).width, 10);
    this.height     = parseInt(window.getComputedStyle(this.elm).height, 10);
    this.top        = parseInt(window.getComputedStyle(this.elm).top, 10);
    this.left       = parseInt(window.getComputedStyle(this.elm).left, 10);
    this.radius     = this.width / 2;
    this.origin     = {
        x : this.left + this.width / 2,
        y : this.top + this.height / 2,
    };
    this.angle = 0;
    this.rotation   = function () {
        if (this.angle >= 360) { this.angle = 0; }
        this.angle += 1;
        this.elm.style.webkitTransform = "rotate(" + this.angle + "deg)";
    };
    this.rotate = function (mthd) {
        setInterval(mthd, 100);
    }
}
var moon = new Mass("moon");
moon.rotate("moon.rotation()");


The issue I'm having is this moon.rotate("moon.rotation()");. There are two methods, and the way I call them is um.. weird? I want to use a single method to rotate the object. Like moon.rotate().

I tried a few ways to put the code in just one method, but couldn't figure it out.

Fiddle

Even though I'm done with the problem mentioned in this question, I wanted to add something, the full code. In case if anyone is interested.

Fiddle

Solution

For one, you don't want to pass a string to setInterval(); better to use a function. You can bind that function to the object's context, so it'll call the right thing.

For another, aim for less ambiguous names. rotation should probably be updateRotation, since that's what it does. And rotate should likely be startRotating (and they you could have an equivalent stopRotating).

That be something like:

this.startRotating = function () {
  var self = this; // to maintain the object's context
  setInterval(function () {
    self.updateRotation();
  }, 100);
};


However, if you're planning to add more planets etc., it would probably be nicer to have one central timer that calls an update function on all your objects.

And I'd also define a "proper" prototype for all this, and extract some general logic/avoid repetition:

// extract this logic, since it's generally useful
function getElement(element) {
  if (typeof element == "string") {
    return document.getElementById(element);
  }
  return element;
}

function Mass(element, angularVelocity) { // added the rotation speed as an arg
  var computedStyle;

  this.element = getElement(element);

  // fetch this once
  computedStyle = window.getComputedStyle(this.element);

  this.width  = parseInt(computedStyle.width, 10);
  this.height = parseInt(computedStyle.height, 10);
  this.top    = parseInt(computedStyle.top, 10);
  this.left   = parseInt(computedStyle.left, 10);
  this.radius = this.width / 2;

  this.origin = {
    x: this.left + this.radius, // you just calculated the radius; might as well use it here
    y: this.top + this.radius
  };

  this.angularVelocity = angularVelocity;
  this.rotation = 0;
}

// make nice prototype
Mass.prototype = {
  update: function () {
    this.rotation = (this.rotation + this.angularVelocity) % 360;
    this.element.style.webkitTransform = "rotate(" + this.rotation + "deg)";
  }
};

// do the timer magic outside the object
var moon = new Mass("moon", 1.0),
    timer;

timer = setInterval(function () {
  moon.update();
}, 100);


fiddle

Code Snippets

this.startRotating = function () {
  var self = this; // to maintain the object's context
  setInterval(function () {
    self.updateRotation();
  }, 100);
};
// extract this logic, since it's generally useful
function getElement(element) {
  if (typeof element == "string") {
    return document.getElementById(element);
  }
  return element;
}

function Mass(element, angularVelocity) { // added the rotation speed as an arg
  var computedStyle;

  this.element = getElement(element);

  // fetch this once
  computedStyle = window.getComputedStyle(this.element);

  this.width  = parseInt(computedStyle.width, 10);
  this.height = parseInt(computedStyle.height, 10);
  this.top    = parseInt(computedStyle.top, 10);
  this.left   = parseInt(computedStyle.left, 10);
  this.radius = this.width / 2;

  this.origin = {
    x: this.left + this.radius, // you just calculated the radius; might as well use it here
    y: this.top + this.radius
  };

  this.angularVelocity = angularVelocity;
  this.rotation = 0;
}

// make nice prototype
Mass.prototype = {
  update: function () {
    this.rotation = (this.rotation + this.angularVelocity) % 360;
    this.element.style.webkitTransform = "rotate(" + this.rotation + "deg)";
  }
};

// do the timer magic outside the object
var moon = new Mass("moon", 1.0),
    timer;

timer = setInterval(function () {
  moon.update();
}, 100);

Context

StackExchange Code Review Q#47155, answer score: 2

Revisions (0)

No revisions yet.