[WIP] SMF Coding Guidelines

[KGIII]

Except, well, the links you posted aren't underlined until mouseover... I agree that they MUST be easily spotted.

[Joshua Dickerson]

Maybe I should change that to "decoration". So long as they are clearly defined. For instance if you were to click the following link or if you were to click this link

[KGIII]

I'd go with something like "clear decoration" or "easily understood navigational links" or something perhaps. Just my two cents.

[B Patterson]

Files

When you are displaying an index of information to be displayed to a user such as BoardIndex.php and MessageIndex.php, the file should end with 'Index'.

Probably needs to be reworded to something like:

When an index of information is to be displayed to a user, such as BoardIndex.php and MessageIndex.php, the file should end with 'Index'.

Queries

   You may not use UNION, SET PASSWORD, BENCHMARK, subselects, or comments in a query. As usual, there is an exception that you should almost never do as it allows for security vulnerabilities:
$modSettings['disableQueryCheck'] = 1;

What's the exception?  I should never what... enable the "disableQueryCheck" setting?  Why? (I don't mean, tell me, but explain it, make a sub-point about the security vulnerability so users can read it.)

   When selecting columns, only select those which you are going to use. Do not use SELECT *.

EXCEPT when doing "SELECT COUNT(*) AS mycount FROM ..." as typically that form is faster than "SELECT COUNT(ID_SOMETHING) AS mycount FROM ...".

[Joshua Dickerson]

Everyone is coming up with excellent points. When I get some time I am going to update this. Might be a couple of weeks though.

[Daniel15]

EXCEPT when doing "SELECT COUNT(*) AS mycount FROM ..." as typically that form is faster than "SELECT COUNT(ID_SOMETHING) AS mycount FROM ...".

Technically, that's not a SELECT *, as you're not selecting all columns. You're simply passing * to the COUNT function. A SELECT * would be something like:
SELECT *, COUNT(*) AS mycount FROM ...

The COUNT(column_name) syntax is useful when you have a GROUP BY clause...

[Dannii]

I think maybe we should add a few more (problems I've come across from mods that conflict with mine):
Don't directly access $_REQUEST['board'] or $_REQUEST['topic'], use $board and $topic instead.
Don't overload the display action (ie, no action) to do stuff other than displaying. Make a new action instead. You can overload existing actions, but only by adding a subaction.

[Joshua Dickerson]

Can you give me an example of overloading? Not totally sure of what you mean. Good point about $board and $topic.

[Dannii]

The topic ratings mod puts its code directly into Display.php using a new variable rather than a new action. It's caused problems because my pretty URLs mod doesn't know that the user is actively trying to do something. It's not just my mod though, I think a general principle should be that messageindex and display are just for reading, and not for any user actions. If PHP had better support, display and messageindex would be accessed only by GET, while the other actions only by POST or PUT.

[karlbenson]

I think a general principle should be that messageindex and display are just for reading

Actually these days with web2.0, I'd expect alot MORE user interactive stuff than there has been before.

I agree though, it may be best to get a handle on it now with suggested 'correct/incorrect' ways of doing it.

[Dannii]

More user interaction is great, but not with GET
It's a shame PHP isn't suited for RESTful web apps.

[karlbenson]

gotcha!

[koji]

"There are some special variables that we use such as $modSettings, $context,
$sourcedir, etc. You can find out more about those in the documentation. Use those
variables before creating your own."

Where do we find out what these legendary variables are?

[Natoka]

very nice coding guidelines, really helpful, hopefully everybody follows them

what I am missing to some extent is some notes about languages / internationalization, since smf  has quite a nice support to do that, but I had to read the source code of several mods in order to get an understanding how to do that. Plus I am still a bit puzzled how I am going to do it, because there are several ways to do it. Additionally it would be nice to have some standards in that area too, so that translators will have an easier job to include mod xyz in their translation.

[jackey]

hey folks!
just started looking round here for some info about modding smf and saw this thread here.. so after i have spent half an hour searching the starting post for any links, doing a thorough forum search looking for those mysterious coding guidelines.. just yielded http://docs.simplemachines.org/index.php?topic=218, where it says it isnt up to date and a link to this topic..
dunno if i am missing something here, but from what i read in this topic, the guidelines could be really useful, so i would appreciate if someone would reveal to me the whereabouts of this magical document everybody praises so highly...

[B Patterson]

Attachments in the first post of this thread by groundup  link

[jackey]

well now i am REALLY confused.. dunno what happened there, maybe i failed to reload the page after i logged in or something, but the links definitely where not there when i looked there three times.. really sorry for acting like some newb here 

anyway, great work there, really covers everything i was unsure about.. are there any changes regarding smf 2.0beta (except those new database query functions) i should consider when coding a mod?

[B Patterson]

Keep in mind things may change   It's still a beta.  I'm willing to bet not much is going to change; however, things may change to fix bugs or fix security holes, so just keep up on the changelog during future beta releases.

[BizB]

You must be logged in to see the attachments' links.

[PJLawrence]

In the coding guide lines (smf coding guidelines.pdf) it states the following..

Do not indent statements in the global scope (not inside of a class, function, statement)
(see the example in Appendix B)

However, Appendix B does not seem to contain this information.
Or is it just me and I've missed it?