4
\$\begingroup\$

Unfortunately I get 3 warnings at jslint. Could someone please help?

I already tried a lot, but didn't get it yet.

Here's a jsfiddle: http://jsfiddle.net/KYjF9/21/

JSlint options:

/*jslint browser: true indent: 2 */ /*global $ jQuery alert*/ 

JavaScript:

var Typing = function (el, toType, period) {
  "use strict";
  this.toType = toType;
  this.el = el;
  this.loopNum = 0;
  this.period = parseInt(period, 10) || 2000;
  this.txt = '';
  this.tick();
  this.isDeleting = false;
};

Typing.prototype.tick = function () {
  "use strict";
  var fullTxt = this.toType[this.loopNum % this.toType.length];

  if (this.isDeleting) {
    this.txt = fullTxt.substring(0, this.txt.length - 1);
  } else {
    this.txt = fullTxt.substring(0, this.txt.length + 1);
  }

  $(this.el).html('<span class="wrap">' + this.txt + '</span>');

  var delta = 300 - Math.random() * 100;

  if (this.isDeleting) { delta /= 2; }

  if (!this.isDeleting && this.txt === fullTxt) {
    delta = this.period;
    this.isDeleting = true;
  } else if (this.isDeleting && this.txt === '') {
    this.isDeleting = false;
    this.loopNum = this.loopNum + 1;
    delta = 500;
  }

  setTimeout(this.tick.bind(this), delta);
};

var data = {
  "text": {
    "list": ["hello", "hallo", "hola", "what", "ever"],
    "period": "3000"
  }
};

$(document).ready(function () {
  "use strict";
  var toType = data.text.list;
  var period = data.text.period;
  $(".js").each(function () {
    if (toType) {
      new Typing(this, toType, period);
    }
  });
});
\$\endgroup\$
  • \$\begingroup\$ The code works! It's just not perfectly valid in jslint. \$\endgroup\$ – fooness Mar 28 '14 at 22:50
  • \$\begingroup\$ @Mat'sMug AFAICT the code works as intended; it's just giving off some linter warnings \$\endgroup\$ – Flambino Mar 28 '14 at 22:51
  • \$\begingroup\$ Close vote retracted, but the question could be reworded to make it clearer that OP is looking for a review, not for specific help fixing/removing/addressing those warnings. \$\endgroup\$ – Mathieu Guindon Mar 28 '14 at 22:53
  • 1
    \$\begingroup\$ re-post of codereview.stackexchange.com/questions/45516/… ? \$\endgroup\$ – Kvothe Mar 28 '14 at 23:01
  • \$\begingroup\$ Whelp, now I'm voting to close. @Kvothe is correct; this seems like a repost. It's not an exact duplicate, but it's close enough that one of two questions should probably be closed \$\endgroup\$ – Flambino Mar 28 '14 at 23:29
3
\$\begingroup\$

I suppose you're talking about these 3 JSLint warnings:

Combine this with the previous 'var' statement.
  var interval = 300 - Math.random() * 100;

Combine this with the previous 'var' statement.
  var interval = 3000;

Do not use 'new' for side effects.
      new Typing(this, toType, interval);

The first 2 are suggestions to combine multiple var declarations into one. For example, instead of:

var x = 3;
var y = 4;

The suggestion is to do like this instead:

var x = 3, y = 4;

The 3rd one is about instantiating something without storing the reference somewhere. This page explains why this is not recommended, in particular:

By not assigning the return value of a constructor to something you will lose the reference to that instance. Generally, by constructing an instance you would want to keep that reference, whether to use again later or for "internal" use as part of a comparison. What's the point of constructing something you are going to throw away as soon as it's been created?

If you have a constructor function that performs work beyond simply setting up an instance, and you are calling that constructor just for these "side effects", consider reworking your code to allow you to call the function normally, without the new operator.

Personally I disagree with the suggestion to combine the var statements, but agree with the 3rd warning.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.