Is there a channel for code reviews or do I just post it here? I am trying to figure out if there is a better way to do this then a bunch of if statements inside other if statements followed by the same thing for the else statements doing the same. Here is a

snipplet

of the code https://bitbucket.org/ookma-kyi/workspace/snippets/rE7agp .

Instead of

if ( structKeyExists( url, 'user' ) && structKeyExists( url, 'token' ) ) {

...
} else {
	event.setView( "resetpassword/malformedurl" );
}

You can do:

if ( not structkeyexists(url, "user") or not strucketexists(url, "token") ){
    event.setView( "resetpassword/malformedurl" );
    return;
}

and so on. Whenever a requirement is not met do something and return. If a requirement is met then it will continue to the next thing.

First things first. Get yer indentation sorted out and repost the code. No-one should have to re-indent someone else's code before reviewing it. Second: @Daniel Mejia is spot on. As a rule of thumb: every subsequent line of code in a logic block should be considered an increase of code smell. Every increased level of indentation in your code should be considered an exponential code smell. Obvs there will always be a need for some logic blocks in your code, but making them bigger / deeper should be making you cringe every time you do so. Also: it helps if you consider

else

to be a code smell too. It's seldom necessary with well organised code. Extract pieces of logic into their own functions, and early-return wherever you can.

👍

For comments that you write in your code, let me add the "what this code does" is normally not needed if the code you have written is (what you / other coder would call - well written). consider the following (obviously bullshit) function;

public numeric plusThree(numeric numberToAdd) {
    return arguments.numberToAdd + 3;
}

The function signature tells you "practically" everything you need to know. • It takes a single argument which is a number • It is going to return a number. • It is called "plusThree" (so its likely going to add 3 to a number you provide!) It really doesn't need any comments at all. However what I find is normally missing is the why (and or ) the when? (and for this point, I would say that every programmer, has their own opinion on what to write, where to write it how much to write about it. My opinion follows - take it with a grain of salt - and use the parts that resonate with you...) In our code we have some spectacularly long functions - they "could" be broken up into smaller functions. "Good design / good practice" says that we should make them as small and discreet as possible - so that it can be read as easily as the function example, above. It (our function that I am thinking of) also has some "deeply" nested logic blocks - that overall from the start of the function to the end result - are quite complex. (again it could be rewritten to "automatically" make some parts instantly clearer to the reader / maintainer - but the effort to "fix" bad code vastly / logarithmically outweighs getting code "right" in the first place. For our function. 1. It takes a set of inputs. 2. It makes decisions based on what inputs are provided and which are missing. 3. It makes more decisions based on what the values are - that were provided 4. Does "stuff" 5. And then dependent on the outcome of "stuff" - does other stuff 6. Depending on the outcome of "stuff" - it might / might not recursuvely - call itself. 7. And then gives the final result. Above, for readability / maintainability - I would like to have seen 3 separate functions; • 1-3 • 4 • 5 But we have a lot of work to do and manpower, in the available time-frame is not infinite. And it currently works. Every time you change something that works - there is a risk. Sometimes the change is superficial, so the risk is so close to zero that you might say there is no risk. But humans are not machines and make mistakes all the time, so any code change is NEVER a zero risk prospect. So we live with our 1400 line function, because; • It takes a set of inputs and gives a correct result, for those inputs. Anyway, enough of the excuses and back to my point... Writing comments like;

// I add 3 : for X+3
// I loop through the query : for loop (var person in local.QueryEmployees){}

Those comments above really aren't needed. The code tells you what it is doing, plainly. BUT if you think they're going to help you / a co-worker, next time you have to read / change the code - add them in. As your confidence and knowledge grows you'll just find that you don't need the "simple" comments any more. What I find is missing LOTS of times in code that I end up having to maintain is the; • Why is this code even here? • When does it get used? (again only my opinion) But I like to add comments like the following. It doesn't have any "what" comments in it • // Loop through the array • // If logged in : show the main menu What it does have is a "*WHY*" and a "*WHEN*"

/**
 * The following block of code looks like it is being run in the wrong order.
 * It seems like it should come after the CASE statement.
 * 
 * However we deliberately run it here, because we recursively call THIS function and the 
 * value for "X" could actually be "YYYYY" here - which if you're just reading this code top-to-bottom
 * might not make any sense to you all.
 * 
 * If "X" = "YYYYY" : we want to treat it THIS way.
 * Otherwise this is the first run-through of this function and we want to treat "X" - just as 
 * you're reading / expecting things to work.
 */

For me : the above "kind" of comment is the most important type to have in your code. And every place I have worked - always seems to have this type of documentation missing from their code-base. My earlier work - most certainly has some rubbish unneeded comments and lots of code that really could have done with a WHY and WHEN. At the time you are writing the code - you KNOW exactly why you have it making THIS decision, for THIS value. It is obvious, at that time, because that's why you're writing THAT code. But months / even years later... given all the other work you have also done... will you still be so in-tune with that old code? Think of it as a FAVOUR to your future-self. Document the "when and why". Don't bother with the "how" - unless it is a particularly "tricky / complicated" code to understand. Best of luck - and keep asking questions!

I like the idea of a code review channel as long as it has rules/guidelines for giving feedback.

1) Adam is not allowed to say "where are your tests?" ...

Adam is allowed to say, buy me a beer!