Amendment to coding style

[norv]

https://github.com/SimpleMachines/smCore/wiki/Coding-Standards

Proposed amendment: opening bracket on the same line for conditional blocks. i.e.
if ($condition) {
// do stuff
}
respectively,
while (true) {
// do even more interesting stuff
}

Obviously, it saves space, and it is still very readable. It is a style used for a long time by many projects, and allows one to see more on a page, while still catching it at a glance.
Not strictly related, it may even allow us to have brackets for one-line statements, while not agglomerating the view anymore (by much).

[mikemill]

Allman is vastly superior. Seriously, having the brackets line up is of help. Unless you are in a 25x80 terminal that extra line is worthless.

[norv]

LOL. I have just argued somewhere on the interwebz, that to dare touching HolyCodingStyle(tm) is a crime... no matter how small or innocent or useful the change is...

On the matter at hand: It is Allman, it is only for conditionals. :)
It's very very easy to distinguish, and many editors/IDEs and projects do it.

Open to discussion, of course if you folks are uncomfortable with it, it ain't happening. I am fine with both, really, just think we might consider to align to a common less-wasting-space style.

[joshuaadickerson]

I agree with Mike.

[Fustrate]

I concur with Mike and Joshua.

[emanuele45]

/me dislikes wastes :P

The one proposed by Norv is the style I used when I started coding in php, then when I started coding for SMF I had to adapt to the Allman style (Spuds's fault BTW :P) and my comment was:

NOOO!!!!
Please not this one... :'(
It's such a waste of space when functions or blocks are collapsed... :'(

Now I don't really care one or another.

[IchBin]

I'd be fine with the swtich, but I can code in either one it doesn't bother me like some of you pre-madona's! ;)

[mikemill]

*prima donna

[norv]

Just a quick one more. I'll use still this issue, for simplicity (or laziness). How about exceptions? There isn't really an (established) style yet since we have just started to use them. According to the same all-newlines habits, it would be:

Version 1.

try
{
   $connection->query("stuff");
}
catch (Exception $e)
{
   Debug::log ($e->getMessage());
}

vs. version 2.

try {
    $connection->query("stuff");
}
catch (Exception $e) {
   Debug::log ($e->getMessage());
}

I'm open to any option, including all-newlines for conditionals too, but exceptions ... excepted. :D. Because for them there's no choice on the {}.

[IchBin]

You totally missed the joke Mike. I was actually calling you madona's (in reference to you guys acting like girls instead).

[mikemill]

Why is there no choice for exceptions Norv? Figure that the try block could be the entire function/method's body you'd still want to line them up. Catches are generally shorter but no reason why they can't be long either.

[norv]

There's no choice on the blocks - they're blocks surrounded by {}. The coding style we have considers that one-line statements are too much for surrounding them in {}, and exceptions are somewhere in between because of that, for short ones, of course. For longer ones, you get into a similar situation as above. Slightly more obvious than similar: try blocks are immediately recognizable because they end with catch, too, not only ending }.

[norv]

And, oh well. As I said, them HolyCodingStyle wars have many of them victims out there. Figured we might make an exception amongst them victims. :)

It's fine either way with me. (on these)
In any case, I'm interested to make sure I know very well your positions on readability, common styles, and flexibility thereof.

For your information (without any implication), in case it picks your curiosity, this is a list of how they're done out there:
22 projects coding style notes

[holy war mode] Although, many are space lovers :P [/holy war mode] ;)

[Fustrate]

Just as a note, PSR-2 is a fucking joke.

[mikemill]

I think you mean PSR is a joke.

[Fustrate]

The FIG idea is good, the execution is bad, PSR-0 was good, PSR-1 is alright, PSR-2 sucks donkey balls.

[mikemill]

When I saw "Method names MUST be declared in camelCase." it was all over for PSR-1. I would have been ok with SHOULD but MUST is just stupid.

I saw a lot of bad stuff with PSR-0 (including a severally broken example)

[norv]

Figured we might make an exception amongst them victims. :)

Nope. We make no exception. PSR-2 claims CS rulez (from the outside) => it won't be taken seriously at best, and get expletives at worst. :)
Noted.

Yeah. I can only agree this kind of stuff in particular in PSR-2 shouldn't have been the target. It's called: holy wars temptation, there for everyone.

For you who may not be aware yet, there is work on caching interfaces and http requests/responses/client, to eventually get to a set of standard interfaces that PHP frameworks/components may be interested to implement in their projects.
Hopefully PSR-Cache and perhaps PSR-Http (freely naming), will prove acceptable to people, they would be very useful as framework/component interoperability standards. Will come back on these.

About PSR-0. I have chosen the PSR-0 extended version, meaning allowing to add multiple paths for a namespace, and multiple top level namespaces overall (for the entire application). Also, comparing to other implementations in the wild, the autoloader doesn't require files. It is NOT acceptable to get an E_COMPILE_ERROR from class_exists().
Only, a development version will throw exceptions. As guideline though, a class loader should play nice with others in the stack.

One thing about PSR-0 is that it's more important when it comes to the need to interoperate: if smCore "ships" a module/part designed fully as a component (meaning with no dependencies), such as Cache almost can be, for example, it is relevant for the receiving application/framework to be able to have the classes autoloaded without the need to do anything in particular. That basically only requires a relevant top level NS, one class per file with the appropriate name, and correspondence between sub-namespaces tree and the structure on the filesystem. Which are all fine.
The other way around is even easier to fit: if I want to autoload Twig or Fustrate's Twig-based engine, all I should need to do is to drop it in the /library folder, at most register it. That only means that the autoloader is capable to handle PSR-0 compatible components.

These two requirements do not cover all scenarios. These are however the strictly needed targets, and imply we continue to respect the necessary for them, which is what we're doing anyway: a class named "smCore\Exception" is in the file "Exception.php", in the directory "smCore", nothing fancier. "smCore" is the namespace, and it maps directly to a directory on the filesystem, where the file will be found. As we're getting used to namespaces, it's the common sense choice anyway too.

[joshuaadickerson]

Just to note, when I wrote those coding standards down I looked at what was already common for us to do and then looked at other places and things that we wanted to do. There is a lot of thought in them but that doesn't mean they shouldn't change. In fact, there is a lot in there that I would say should go now.

When I was thinking about them, I thought how could I do make them standard enough and easy enough to understand that I could write a lexical parser to test the code's standards validity.

[norv]

Actually, there are many missing. Details that affect code consistency, which SMF either has them inconsistently done, either it has them consistently or close to, but they're not documented. Such as space or no space between function/method name and opening bracket, space or no space inside brackets of a condition, etc.

In fact, there is a lot in there that I would say should go now.

Details, please.

I could write a lexical parser to test the code's standards validity.

Go ahead. There are tools available to eventually pick and modify accordingly; among the latest I've seen that you were around too are those created and discussed by PSR/FIG: a fixer for PSR-1 and -2, sniffers for PEAR and PSRs. You'd need to check the rules they define and modify or implement anew the appropriate ones for us, as we go.

[mikemill]

One rule that is clearly missing is that AS should be in all caps in a foreach :P

[Fustrate]

Only if we also capitalize the EACH in forEACH, ya'know, to distinguish it from for.

[mikemill]

That's brilliant!

[Fustrate]

Just you wait! All echo statements must use parentheses! You must assign your return value to a variable before returning it! ALL FUNCTION NAMES MUST START WITH THE LETTER V!

[mikemill]

You must only use do while loops

[Fustrate]

On a serious note,

opening bracket on the same line for conditional blocks

means we couldn't quickly comment out the conditional line, like this:

// if (user is dead)
{
    blah blah blah
}