Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lots of redeclared variables in the Spectrogram plugin #1138

Closed
glebm opened this issue Jun 16, 2017 · 8 comments
Closed

Lots of redeclared variables in the Spectrogram plugin #1138

glebm opened this issue Jun 16, 2017 · 8 comments

Comments

@glebm
Copy link
Contributor

glebm commented Jun 16, 2017

@akreal

These are the errors I'm getting at the current master:

plugin/wavesurfer.spectrogram.js:276: ERROR - Redeclared variable: bgFill
  274|     loadLabels: function (bgFill, fontSizeFreq, fontSizeUnit, fontType, textColorFreq, textColorUnit, textAlign, container) {
  275|         var frequenciesHeight = this.height;
  276|         var bgFill = bgFill || 'rgba(68,68,68,0.5)';
  277|         var fontSizeFreq = fontSizeFreq || '12px';
  278|         var fontSizeUnit = fontSizeUnit || '10px';

plugin/wavesurfer.spectrogram.js:277: ERROR - Redeclared variable: fontSizeFreq
  275|         var frequenciesHeight = this.height;
  276|         var bgFill = bgFill || 'rgba(68,68,68,0.5)';
  277|         var fontSizeFreq = fontSizeFreq || '12px';
  278|         var fontSizeUnit = fontSizeUnit || '10px';
  279|         var fontType = fontType || 'Helvetica';

plugin/wavesurfer.spectrogram.js:278: ERROR - Redeclared variable: fontSizeUnit
  276|         var bgFill = bgFill || 'rgba(68,68,68,0.5)';
  277|         var fontSizeFreq = fontSizeFreq || '12px';
  278|         var fontSizeUnit = fontSizeUnit || '10px';
  279|         var fontType = fontType || 'Helvetica';
  280|         var textColorFreq = textColorFreq || '#fff';

plugin/wavesurfer.spectrogram.js:279: ERROR - Redeclared variable: fontType
  277|         var fontSizeFreq = fontSizeFreq || '12px';
  278|         var fontSizeUnit = fontSizeUnit || '10px';
  279|         var fontType = fontType || 'Helvetica';
  280|         var textColorFreq = textColorFreq || '#fff';
  281|         var textColorUnit = textColorUnit || '#fff';

plugin/wavesurfer.spectrogram.js:280: ERROR - Redeclared variable: textColorFreq
  278|         var fontSizeUnit = fontSizeUnit || '10px';
  279|         var fontType = fontType || 'Helvetica';
  280|         var textColorFreq = textColorFreq || '#fff';
  281|         var textColorUnit = textColorUnit || '#fff';
  282|         var textAlign = textAlign || 'center';

plugin/wavesurfer.spectrogram.js:281: ERROR - Redeclared variable: textColorUnit
  279|         var fontType = fontType || 'Helvetica';
  280|         var textColorFreq = textColorFreq || '#fff';
  281|         var textColorUnit = textColorUnit || '#fff';
  282|         var textAlign = textAlign || 'center';
  283|         var container = container || '#specLabels';

plugin/wavesurfer.spectrogram.js:282: ERROR - Redeclared variable: textAlign
  280|         var textColorFreq = textColorFreq || '#fff';
  281|         var textColorUnit = textColorUnit || '#fff';
  282|         var textAlign = textAlign || 'center';
  283|         var container = container || '#specLabels';
  284|         var getMaxY = frequenciesHeight || 512;

plugin/wavesurfer.spectrogram.js:283: ERROR - Redeclared variable: container
  281|         var textColorUnit = textColorUnit || '#fff';
  282|         var textAlign = textAlign || 'center';
  283|         var container = container || '#specLabels';
  284|         var getMaxY = frequenciesHeight || 512;
  285|         var labelIndex = 5 * (getMaxY / 256);

plugin/wavesurfer.spectrogram.js:303: ERROR - Redeclared variable: index
  301|             var freq = freqStart + (step * i);
  302|             var index = Math.round(freq / this.sampleRate / 2 * this.fftSamples);
  303|             var index = Math.round(freq / (this.fftSamples / 2) * this.fftSamples);
  304|             var percent = index / this.fftSamples/2;
  305|             var y = (1 - percent) * this.height;

@agamemnus
Copy link
Contributor

Well, it's not a Javascript error.

@glebm
Copy link
Contributor Author

glebm commented Jun 16, 2017

It's still a good thing to fix, as it doesn't make sense to redeclare variables

@agamemnus
Copy link
Contributor

agamemnus commented Jun 16, 2017

Sometimes, though, it is better to declare variables twice, for purposes of clarity... for example:

if (reallylonghardtonamethinga) {
 var b = x
} else {
 var b = y
}

Of course you can just declare var b at the top, but then you are adding an extra line that does nothing.

@agamemnus
Copy link
Contributor

(Although here arguably they probably shouldn't be there, and the last one is a straight up ???).

@glebm
Copy link
Contributor Author

glebm commented Jun 16, 2017

Perhaps, but in this case it's

var a = a || 1

which does not make sense unless a is already declared

@glebm
Copy link
Contributor Author

glebm commented Jun 16, 2017

Yeah the last one is confusing.

@akreal
Copy link
Contributor

akreal commented Jun 17, 2017

There are no good reasons for these redeclarations.

@agamemnus
Copy link
Contributor

agamemnus commented Jun 17, 2017 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants