[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
User avatar
bantu
3.0 Release Manager
3.0 Release Manager
Posts: 557
Joined: Thu Sep 07, 2006 11:22 am
Location: Karlsruhe, Germany
Contact:

Re: [RFC|Accepted] Coding Guideline Modifications

Post by bantu » Mon Dec 20, 2010 10:37 pm

Yes, sure. But can we please get it done ... like now?

User avatar
naderman
Product Manager
Product Manager
Posts: 1727
Joined: Sun Jan 11, 2004 2:11 am
Location: Karlsruhe, Germany
Contact:

Re: [RFC|Accepted] Coding Guideline Modifications

Post by naderman » Fri Jan 14, 2011 8:13 am

Where is the pull request? :)

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

Re: [RFC|Accepted] Coding Guideline Modifications

Post by Oleg » Wed Feb 16, 2011 2:16 am

IRC wrote: (12:01:53) naderman: are SPL Exceptions 5.3+?
(12:01:56) naderman: I guess ...
(12:02:10) nn-: naderman: check class constructors
(12:02:35) naderman: " PHP 5.1 "
(12:02:35) naderman: :)
(12:02:49) naderman: so we can use stuff like SPL's RuntimeException
(12:02:59) naderman: instead of creating our own exception class for every little detail
(12:03:18) nn-: difference between runtime exception and exception being?
(12:03:27) naderman: (there are a bunch of exception class defined in http://www.php.net/~helly/php/ext/spl/c ... ption.html )
(12:03:34) naderman: well it's bad practice to throw generic exceptions
(12:03:43) naderman: because if you want to catch one you then catch all of them
(12:03:48) naderman: even ones you might not have expected
(12:03:49) nn-: we should have a hierarchy, no question
(12:04:15) naderman: generally having one superclass for all exceptions causes trouble because you shouldn't be catching all kinds of exceptions anyway
(12:04:30) naderman: err disregard previous url
(12:04:30) naderman: http://www.php.net/~helly/php/ext/spl/annotated.html
(12:04:32) nn-: ah, without spl we only have one base class to wok with?
(12:04:32) naderman: is what I meant
(12:04:44) naderman: there is one Exception class in php
(12:04:47) naderman: you can define your own
(12:04:59) naderman: but SPL defines a couple of standard ones so you don't need to do that all the time
(12:05:29) nn-: seems sane in principle
(12:05:37) naderman: so basically the process is: I need to throw an exception here -> check if SPL has an appropriate exception class - if yes use, if no create your own
(12:06:32) nn-: agreed, and remember to check version number ;)
tl;dr: Instead of throwing Exception, throw meaningful derived classes. Before creating new classes check if SPL already has a suitable exception class, and be sure to check that it was added in a sufficiently old php version.

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

Re: [RFC|Accepted] Coding Guideline Modifications

Post by Oleg » Wed Feb 16, 2011 2:40 am

Also, it was brought up that we should standardize on requiring or omitting @return void.

TerryE
Registered User
Posts: 95
Joined: Sat May 23, 2009 12:24 am
Contact:

Re: [RFC|Accepted] Coding Guideline Modifications

Post by TerryE » Mon Mar 21, 2011 12:47 pm

The inline documentation tags clearly have a syntax. Which documentation tool does phpBB use/proposing to use? It's just that if we want to encourage inline documentation, then it would be nice to know which syntax to conform to -- e.g. phpDocumentor, Doxygen, or ???

[edit] Having had a scan, it looks like phpDocumentor syntax. I suppose that we all have our own poison, but I find this a lot more difficult to get to grips with than Doxygen. I find the documentation of phpDocumentor difficult to follow, which is sad because I would expect a package which is designed to produce good documentation to have good documentation.

Do you use a standard template ?

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

Re: [RFC|Accepted] Coding Guideline Modifications

Post by igorw » Mon Mar 21, 2011 7:53 pm

We use phpdoctor to build our API docs. If you have phing, you can go into the build directory (within the git repo) and run "phing docs", else "phpdoctor phpdoc-phpbb.ini" should do the job too.

User avatar
naderman
Product Manager
Product Manager
Posts: 1727
Joined: Sun Jan 11, 2004 2:11 am
Location: Karlsruhe, Germany
Contact:

Re: [RFC|Accepted] Coding Guideline Modifications

Post by naderman » Wed May 25, 2011 3:29 am

nn- wrote:Also, it was brought up that we should standardize on requiring or omitting @return void.
I think we should aim to always document the return value to avoid confusion between undocumented return values and void return values. However since a function without return value is treated as a function with return value null, I think we should always use @return null. This will also help PHP programmers who don't necessarily know the meaning of "void" in this context ;-)

User avatar
bantu
3.0 Release Manager
3.0 Release Manager
Posts: 557
Joined: Thu Sep 07, 2006 11:22 am
Location: Karlsruhe, Germany
Contact:

Re: [RFC|Accepted] Coding Guideline Modifications

Post by bantu » Wed May 25, 2011 10:36 am

naderman wrote:
nn- wrote:Also, it was brought up that we should standardize on requiring or omitting @return void.
I think we should aim to always document the return value to avoid confusion between undocumented return values and void return values. However since a function without return value is treated as a function with return value null, I think we should always use @return null. This will also help PHP programmers who don't necessarily know the meaning of "void" in this context ;-)
+1

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

Re: [RFC|Accepted] Coding Guideline Modifications

Post by callumacrae » Fri Aug 19, 2011 12:16 pm

I don't know whether this is official or not, but me and igorw have both been parentheses on the same line with anonymous functions in Javascript:

Code: Select all

if (sth)
{
	function run(fn)
	{
		return fn();
	}
	run(function() {
		console.log('test');
	});

	//as opposed to:
	run(function()
	{
		console.log('test');
	});
}
Second one looks extremely messy. Can something be added to the coding guidelines about this?
Made by developers, for developers!
My blog

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

Re: [RFC|Accepted] Coding Guideline Modifications

Post by igorw » Fri Aug 19, 2011 1:04 pm

Yes, I would even add a space after function for lambdas, as douglas crockford does.

Code: Select all

function foo(a, b)
{
    // do something
}

foo(function (a, b) {
    // do something
});

Post Reply