[RFC|Accepted] Coding Guideline Modifications

These requests for comments/change have lead to an implemented feature that has been successfully merged into the 3.1/Ascraeus branch. Everything listed in this forum will be available in phpBB 3.1.
Post Reply
Oleg
Posts: 1150
Joined: Tue Feb 23, 2010 2:38 am
Contact:

Re: [RFC|Accepted] Coding Guideline Modifications

Post by Oleg »

igorw wrote:Yes, I would even add a space after function for lambdas, as douglas crockford does.
I think almost nobody does that. So I'd rather not do it either.

For where to place the braces, as much as I hate braces on their own lines if this is the convention we use in php code it makes sense for consistency to use the same convention in js. Unless we get rid of inline js, then I would probably support moving braces up onto the previous line.

User avatar
callumacrae
Former Team Member
Posts: 1046
Joined: Tue Apr 27, 2010 9:37 am
Location: England
Contact:

Re: [RFC|Accepted] Coding Guideline Modifications

Post by callumacrae »

Oleg wrote:
igorw wrote:Yes, I would even add a space after function for lambdas, as douglas crockford does.
I think almost nobody does that. So I'd rather not do it either.
I agree, and it looks ridiculous when there are no arguments:

Code: Select all

run(function () {
});
Oleg wrote:For where to place the braces, as much as I hate braces on their own lines
I prefer them on their own line in PHP, but with the amount of anonymous functions in JavaScript, it looks stupid to have them on their own line, and stupid to have them inconsistent. I would be all for moving all braces to the same line in the JavaScript, but I can't really see that happening.
Oleg wrote:if this is the convention we use in php code it makes sense for consistency to use the same convention in js. Unless we get rid of inline js, then I would probably support moving braces up onto the previous line.
We're trying to get rid of most of the inline js in 3.1. I don't really understand this last bit of your post, though :-/
Made by developers, for developers!
My blog

igorw
Registered User
Posts: 500
Joined: Thu Jan 04, 2007 11:47 pm

Re: [RFC|Accepted] Coding Guideline Modifications

Post by igorw »

callumacrae wrote:I agree, and it looks ridiculous when there are no arguments
Well that's subjective. phpBB has this habit of making its own standards for everything, but that's ok. The only thing I'm completely against is opening braces of lambdas on their own line:

Bad

Code: Select all

$(function()
{
    // WTF
});
Good

Code: Select all

$(function() {
    // Makes sense
});
For named functions, own line is fine, and would be consistent with our PHP.

Code: Select all

function foo()
{
    // bar
}

Oleg
Posts: 1150
Joined: Tue Feb 23, 2010 2:38 am
Contact:

Re: [RFC|Accepted] Coding Guideline Modifications

Post by Oleg »

I withdraw my objections.

topdown
Registered User
Posts: 12
Joined: Sat Dec 01, 2007 5:04 am

Re: [RFC|Accepted] Coding Guideline Modifications

Post by topdown »

I didn't see anything in here about return always returning something.
So I am bringing it up.

The old code base has a lot of return; in it.
It would be better if it always returned something return true;

User avatar
naderman
Consultant
Posts: 1727
Joined: Sun Jan 11, 2004 2:11 am
Location: Berlin, Germany
Contact:

Re: [RFC|Accepted] Coding Guideline Modifications

Post by naderman »

No that would not be better. If a function has no return value, it should not return some meaningless boolean value. Regardless of that we should document these functions as returning null.

topdown
Registered User
Posts: 12
Joined: Sat Dec 01, 2007 5:04 am

Re: [RFC|Accepted] Coding Guideline Modifications

Post by topdown »

Then why not just return null; ?

At the same time, wouldn't t be better to return early.

Eg.

Code: Select all

/**
 * @param string $str
 * @return null|string
 */
function test_returns($str = '')
{

	if(!$str)
	{
		return null;
	}
	else
	{
		return $str;
	}
}
Instead of

Code: Select all

/**
 * @param string $str
 * @return null|string
 */
function test_returns($str = '')
{

	if($str)
	{
		return $str;
	}

	return null;
}
This is a small picture, but on a larger scale it can conserve resources, tiny amounts, but still.

If the condition doesn't evaluate return null right away and stop processing the rest of the function.

I should also mention that I don't recall off hand of not seeing early returns, just more of a clarification thing.

Oleg
Posts: 1150
Joined: Tue Feb 23, 2010 2:38 am
Contact:

Re: [RFC|Accepted] Coding Guideline Modifications

Post by Oleg »

"return null" suggests that something other than null may be returned in other circumstances. "return" makes it clear that the function never returns a value.

With respect to early returns, there are two arguments that are commonly made:

1. Keeping the number of exit points in a function low (ideally, 1) makes the function easier to follow.

2. Keeping the indentation level as low as possible makes the code easier to read.

These are somewhat contradictory, thus usually it is up to the person writing code to determine when to return early and when not to.

wGEric
Registered User
Posts: 521
Joined: Wed Jun 11, 2003 2:07 am
Contact:

Re: [RFC|Accepted] Coding Guideline Modifications

Post by wGEric »

In regards to the curly braces this code

Code: Select all

return
{
    some: 'object'
}
does not return an object although it appears like it does. So it is usually a good idea to place all curly braces on the same line for everything in javascript so that you don't have any chance of running into that issue.

Also it simplifies things by saying x has to go here in all cases instead of x can go in y place in case z but has to go in b place in case c.
Eric

igorw
Registered User
Posts: 500
Joined: Thu Jan 04, 2007 11:47 pm

Re: [RFC|Accepted] Coding Guideline Modifications

Post by igorw »

Good point, the value of a return statement must always start on the same line.

Post Reply