Hello everyone, 
The short of it is my code sucks.  It sucks in a lot of ways because it's written poorly.  There is a ton of stuff everyone talks about around here that just goes way over my head.  Everyone else seems to relate, but not me.  Thing is this site is predominantly a programming website.  I'd like to get some feedback on my crappy coding style.  I'm also going to start asking a lot more stupid questions, like what the hell are you all talking about half the time?  That last question doesn't need to be answered

Here is my code that I changed on my last blog

initilizingFunction.definePositionPlane = function( unitScale ){ 

	var geometry = new THREE.Geometry();
	var material = new THREE.MeshLambertMaterial( { side: THREE.DoubleSide } );

  	// 'qv(0,0,0)' is short form of 'new THREE.Vector3( 0,0,0 )'
	geometry.vertices.push( qv( 0 , 0 , 0 ) );
  
  	/* the following function creates an 11 x 11 grid of vertices.
  	* the first vertices is located at the center of the grid.
  	* if we were to number the vertices 1 through 121 left from right, up to down
  	* then the first vertices defined would be at the position of the 61st spot in the grid.
  	* as the function cycle through 'i', two 'n' loops of 'l' length add move the location 
  	* of the next declared vertices 'l' left/right depending and 'l' up/down depending.
  	* at the end of both 'n' loops l is incremented and the directions toggle.
  	*/
  
	for( var i=0, j=61, k=61, l=1, q=-1, p=1, b=15; i<121; i+=0 ){
		if( l < 11){

			for( var n=0; n<l; n++ ){
				j += q;
				geometry.vertices.push( qv( ( j - 61 ) * u * b , 0 , ( k - 61 ) * u * b ) );
				i++;
			}
			for( var n=0; n<l; n++ ){
				k += p;
				geometry.vertices.push( qv( ( j - 61 ) * u * b , 0 , ( k - 61 ) * u * b ) );
				i++;
			}
			q*=-1;
			p*=-1;
		
			l++;
		} else {
			for( var n=0; n<10; n++ ){
				j += q;
				geometry.vertices.push( qv( ( j - 61 ) * u * b , 0 , ( k - 61 ) * u * b ) );
				i++;
			}
			i = 121;
		}
	}

	var face = new THREE.Face3( 0 , 10 , 120 );
	geometry.faces.push( face );

	face = new THREE.Face3( 0 , 120 , 110 );
	geometry.faces.push( face );

	geometry.computeFaceNormals();

	var mesh = new THREE.Mesh( geometry , material );
	mesh.visible = true;

	return mesh;

};

What I'd like to know is how would you have written the function?  What kind of comments would you have written?  What 'best company practises' do you follow?
 

I didn't try to analyze your loop, so I'll just assume it's correct and does what you want. One observation I'll offer is that the statements in a 'for' loop are optional, so you don't need to include e.g. 'i+=0' if you don't want anything to happen there. You can just leave that statement out.

Is there any reason you're not using newer features of JS such as 'let' and 'const'? (I'm assuming this is JS.) You may very well have a reason, but if it's just lack of familiarity, you might look into it. In particular, some would argue that 'let' and 'const' should in general be favored over 'var' in modern JS.

One of several reasons for this is that 'var' has sort of unusual and potentially confusing semantics (at least compared to how variables are handled in some other popular languages). I don't immediately see anything in your code that's wrong per se (I'm guessing it runs correctly for you), but it might not be doing exactly what you think it's doing. If you're interested, you might read up on 'var' in JS, and in particular the topic of hoisting.

Also, if you're not already familiar with them, you might look into tools like JSHint, as they'll often point out 'suspicious' code or code that might lead to unintuitive results. There's an online version of JSHint here:

http://jshint.com/

You might try pasting your code there. It'll probably mention some undefined variables, which isn't necessarily a problem since they may be global or otherwise available. But it might also highlight some things you're not aware of.

Since with JS a lot of stuff happens at run time that happens at build time in other languages, tools like JSHint can be invaluable in highlighting possible issues before they manifest. I think it's pretty standard to apply these tools as part of professional JS development (sometimes they're incorporated into automated build systems that combine operations such as linting, concatenation, and minimization).

Another common suggestion is to avoid seemingly arbitrary literals like 11 and 121, and instead use named constants (conceptually constant at least). Your use of the literal 121 is a good example of this. Presumably you're using that value because it's 11 squared, but what if you decide to change the '11' part but forget to change '121'? Using named variables and deriving dependent variables from other variables rather than defining them explicitly will help avoid errors of that sort.

Anyway, those are just a few ideas for you to consider.

thank you for the feedback.  I'm familiar with hoisting and have previously used let and const.  I currently use const with limits, mainly I forget.

Looking at the code though, would it be acceptable for an employer? or say a project partner?  More specifically you?  Or in your case would proper hoisting standards and the use of let and const be enough?

1 hour ago, Awoken said:

Looking at the code though, would it be acceptable for an employer? or say a project partner?  More specifically you?  Or in your case would proper hoisting standards and the use of let and const be enough?

There are probably people here who are involved in hiring JS developers (or developers in general), or have been through that process as applicants, and can therefore speak directly to that. With any luck one of them will see your post and comment.

I can only speculate myself, and offer some opinions.

I think good arguments can be made for the following practice: use 'const' wherever you can, 'let' only where you need mutability, and 'var' never (with the possible exception of creating properties on the global object when the global object isn't available by name), and scope variables as narrowly as is practical. This is assuming of course that you don't have particular reasons for avoiding newer JS features (such as working in an environment that doesn't support them - for example, there are some JS engines that don't support newer features).

To keep this post short I'm not going to elaborate further on that, but a search for e.g. 'javascript const let var' should return plenty of discussion on the topic.

Since you're interested in how others might respond to your code, if you're going to stick with 'var' for whatever reason, it might be advantageous to at least avoid multiple declarations, declarations inside inner scopes, and so on. You may know exactly what you're doing, but to an outside observer those practices might suggest lack of familiarity with JS's idiosyncrasies. Might as well avoid that ambiguity

Another idea is to make a 'clean linting' a minimal bar for your code. In other words, make sure it lints cleanly, or that if it doesn't you know why it doesn't and have reasons for how you're doing things.

Suggestions like the ones I'm offering here can sometimes prompt strong responses in people, so I'll emphasize here that these are just my opinions.

Lastly, here:

var face = new THREE.Face3( 0 , 10 , 120 );
geometry.faces.push( f );

What's 'f'? Since 'face' isn't used, I'm wondering if maybe 'f' is supposed to be 'face'.

22 hours ago, Zakwayda said:

Suggestions like the ones I'm offering here can sometimes prompt strong responses in people, so I'll emphasize here that these are just my opinions.

I'll do some research on linting.

I corrected the code to show 'face' instead of 'f'

Thanks again.