The unofficial coding best practise and code formatting conventions for XBMC
#16
i currently use "real" tabs in vs.net. am i the only one? :d

btw, this is how most switch statements in the window classes are currently written:
Quote:switch
{
case gui_msg_1:
{
onmsg1();
}
break;

case gui_msg_2:
{
onmsg2();
[some other code]
}
break;
}
to discuss, if we keep it that way and add it to the conventions. Smile

greets

bobbin007
Always read the XBMC online-manual, FAQ and search the forum before posting.
Do not e-mail XBMC-Team members directly asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting please make sure you read this first.


Image
Reply
#17
Exclamation 
Maybe slightly off-topic but I like to request that we start requiring basic doxygen tags/comments in XBMC code (including patches). bobbin007 added a few doxygen tags/comments to XBMC CVS a few monts back but it never seem to catched on. BTW, see MPlayer uses it now too Nod

Also, always prefer American English over British English in comments and strings!

Wink
Always read the XBMC online-manual, FAQ and search the forum before posting.
Do not e-mail XBMC-Team members directly asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting please make sure you read this first.
Reply
#18
Sad 
maybe borrow this text/guidelines bellow from mplayer (or parts of it?)?, what do you all think?:
Quote:date: tue, 31 aug 2004 09:14:36 +0900
from: attila kinali <[email protected]>
subject: [mplayer-dev-eng] code documentation guideline draft
to: [email="[email protected]"][email protected][/email]

heyo,

i used 5 free minutes to put a code documentation guideline together.
please review it and tell me anything you dont like or that should
be improved.

attila kinali
-------------- next part --------------
code documentation guidelines
=============================

about this guide
----------------

due to the ever increasing complexity and size of mplayer it became quite hard
to maintain the code and add new features. part of this problem is the lack
of proper documentation what the code in question does and how it is doing it.
to tackle this problem every part of mplayer should follow these guidelines
on what and how code should be documented.


doxygen
-------

mplayer uses doxygen for its code documentation. it generates html files
which contain the specially tagged comment lines from the code including
cross references. to generate it type `make doxygen` in the source root
directory. it will generate the files in docs/tech/doxygen. to clear them
again, you can use `make doxygen_clean`.
for further information about doxygen and its sources please have a look
at their website: http://doxygen.sf.net


what should be documented?
--------------------------

- every function, no matter whether it is globally or just locally used
* what the function does
* all parameters and return values of the function and their valid ranges
* all side effects (to passed parameters, globals etc)
* all assumptions made within the function

- global, file local and important variables
* what it is used for
* its valid range
* where it is set (optional)
* where validity checking is done (optional, mandatory for variables which
are set by something external, eg user parameters, file information etc)

- #define, typedefs, structs
* all global definitions
* all local definitions whos use is not imediatly clear by their name
(as a rule of thumb, it's better to document too much then not enough)
* all dependencies

- non-trivial parts of the code
* tricky parts
* important parts
* special side effects
* assumptions of the code
* string operations and why they are safe

- workarounds
* why they are needed
* how they work
* what side effects they have

if you are unsure whether a comment is needed or not, add one.


how should it be documented?
----------------------------

all comments should be in correct and clear english. any other language is
not allowed. the language used should be kept simple as the code will be
read mostly by non-native speakers. for function and variable documentation,
a style usable by doxygen should be used. see section 3 "documenting the code"
of the doxygen manual for an introduction. a very short overview follows:

doxygen includes only special comments in the documentation, those are:

/// this is a line used by doxygen.

/** this one, too */

/** these
here
of
course,
too */

//! this form can be also used

// this line isn't included in doxygen documentation.

/* neither is this. */

there are a couple of special tags for doxygen:

\brief <one line text>
gives a brief description of a function.
\param <parameter> <text>
describes a specific <parameter>.
\return <text>
describes the return value.
\a <word>
mark next word as a reference to a parameter.
\e <word>
use italic font for the next word.
\b <word>
use bold font for the next word.
\c <word>
use typewriter font for the next word.
\sa <references>
adds a section with crossreferences.
\bug <text>
describe a known bug.
\todo <text>
add a todo list.
\attention <text>
add a section for something that needs attention.
\warning <text>
add a section for a warning.
\anchor <refname>
set an invisible anchor which can be used to create a link with /ref.
\ref <refname> [<text>]
add a link to <refname>.

for a complete list of tags please read section 20 "special commands" of the
doxygen manual.
ps! ffmpeg has now also started using doxygen to document their source code.
Always read the XBMC online-manual, FAQ and search the forum before posting.
Do not e-mail XBMC-Team members directly asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting please make sure you read this first.
Reply
#19
i don't think we'll ever reach an agreement on this thing.

can you sumbit an updated code style text and also put it in cvs?

thanks,

-yuval
Reply
#20
(yuvalt @ sep. 06 2004,17:05 Wrote:i don't think we'll ever reach an agreement on this thing.
-yuval
i agree! there will ever be an agreement about coding standard and formatting.
but... by using a code beautifier, we could increase the quality on this code and make it more readable for every body.

i have been using gc (great code) for a while and it works fine for me. the only switch you desperately need is
"-no-code_decl_move_top-" since the default is to move declarations to the top of the scope and the could ruin the code since also class objects with constructors will be move.
the rest is just cosmetics. my opinion is that gc beautifies to much (read: changes to much), but using a beautifier gives you and your co-programmers so much back that i'm wiling to accept that

if gc is to be used, the gc.cfg file (at least containing "-no-code_decl_move_top-") could be added to cvs root directory.

most of the part of this topics discussion could then be about the gc.cfg file

some words about code beautifier
=========================
q. how to introduce them in a project?
a. check out all code and parse the beautifier through the files, then check in the code without doing any other changes :thumbsup:

q. doesn't a good programmer write code that is more readable than what a beautifier could do?
a. yes, but how many of us does really take us the time of doing that Confusedhifty:

q. could i thrust a beautifier?
a. yes, but first read this how can i trust beautifier programs??!!

also read this from: how to write unmaintainable code :joker:
never beautify: never use an automated source code tidier (beautifier) to keep your code aligned. lobby to have them banned them from your company on the grounds they create false deltas in pvcs (version control tracking) or that every programmer should have his own indenting style held forever sacrosanct for any module he wrote. insist that other programmers observe those idiosyncratic conventions in "his " modules. banning beautifiers is quite easy, even though they save the millions of keystrokes doing manual alignment and days wasted misinterpreting poorly aligned code. just insist that everyone use the same tidied format, not just for storing in the common repository, but also while they are editing. this starts an rwar and the boss, to keep the peace, will ban automated tidying. without automated tidying, you are now free to accidentally misalign the code to give the optical illusion that bodies of loops and ifs are longer or shorter than they really are, or that else clauses match a different if than they really do. e.g.
if(a)
 if(b) x=y;
else x=z;
Reply
#21
Sad 
fyi, mplayer (which xbmc a/v player is based on) has a strict policy against code beautification!, and i have to agree with that when it comes to our mplayer-core in xbmc and for all other code that's been ported from other sources (eg. not written from scartch). if for exampel our mplayer-core code was 'beautified' then it would be harder to update it from the latest mplayer cvs as the code would not be layed-out the same, same goes all other libs and code we use in xbmc that we update quite often (like libcdio, filezilla, ffmpeg, xvid, etc...). only code that can be beautified without consequenses are the xbmc code that has been written from scratch by our own developers, (like the xbmc-gui).
Always read the XBMC online-manual, FAQ and search the forum before posting.
Do not e-mail XBMC-Team members directly asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting please make sure you read this first.
Reply
#22
hi, i just have a quick question, maybe slightly off-topic but i didn't know where else to ask. i read on the c++ faq lite (http://www.parashift.com/c++-faq-lite/fr...l#faq-16.7) that the value returned by the "new" operator should not be checked for zero as it throws on exceptions. this seems to be the way new is called most of the time in xbmc code but then i stumbled upon this msdn bit (http://msdn.microsoft.com/library/defaul...ess_25.asp) which states the opposite.
Quote:if unsuccessful, by default new returns zero.
so which is true, should i check my news or not?
Reply
#23
Sad 
tought i :bump: this, 'even' mediaportal now have guide-lines, maybe we can use that as a base (though obviously remove the c# stuff) Huh
....what do you all think? is anyone (developer) willing to put together a draft of a basic guideline for adding new code and changing old one?
Always read the XBMC online-manual, FAQ and search the forum before posting.
Do not e-mail XBMC-Team members directly asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting please make sure you read this first.
Reply
#24
Exclamation 
i like to bump this discussion once again with a link to a essay titled "comments are more important than code" (which been 'slashdotted')
Always read the XBMC online-manual, FAQ and search the forum before posting.
Do not e-mail XBMC-Team members directly asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting please make sure you read this first.
Reply
#25
Question 
  • XML Filenames - I see you've changed all the files to be lowercase and added underscores in the custom files (views_common.xml etc). A large number of these names are fixed in the XBMC code so can't conform to the underscore naming (DialogYesNo.xml for example) so I'd suggest we don't use the underscores and stick with camel (mixed) case to make them easier to read?

  • Image Filenames - Again there are some images names that are hard-coded into XBMC (such as DefaultFolderBig.png) but the majority of the images we use can be named as and how we like. I would stick with the lower case names with underscores that you have in your zip.

  • Attribute/Element Names - we need to be consistent with naming of things within the xml (such as fonts, includes, viewtype labels etc) and I would propose we use camel case again (without underscores).

  • Info labels/Built In Functions - underscores can't be added in these and having them all lower case makes them more difficult to read so I would go for camel case again.

  • Descriptions - These can be free form (and without underscores replacing spaces).

  • Tabs/Indents - tab width of 2 and convert all tabs to spaces.

  • White Space - Blank lines between distinct code blocks and comments to distinguish code areas (such as a comment block to group together includes relating to video screens etc)-



Also, maybe a brief statements on line endings would be useful?
Reply
#26
Sorry to drege up an old topic, but this thread is still useful for those of us just starting out in the code. I've run into a variety of switch statement formats. One of which seems better than the current documented standard (although with the Quote thingy in the wiki the spaces might have just gone away).

Code:
switch (cmd)
{
case ksavecmd:
  save();
  break;
case kloadcmd:
case kplaycmd:
  close();
  break;
default:
  dialog::handlecommand(sender, cmd, data);
}
Reply
#27
Thanks - yeah, the formatting just went haywire.

Cheers,
Jonathan
Always read the XBMC online-manual, FAQ and search the forum before posting.
Do not e-mail XBMC-Team members directly asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting please make sure you read this first.


Image
Reply
#28
Hi there,

as no one here really responded to the doxygen proposals of Gamester17 and I saw in the wiki a guideline saying one _should_ use doxygen to comment the code: what doc-style for doxygen should one use? Doxygen allows for multiple comment types (see http://www.stack.nl/~dimitri/doxygen/docblocks.html) and I didn't really found any reference of doxygen doc in the code as of yet.
Reply
#29
Tongue

jmarshall has started doing some doxy, see e.g. utils/JobManager.h
Reply
#30
Question 
Hey everyone

I'm currently working on making XBMCs JSON RPC api compliant with the JSON RPC 2.0 specification (see Trac Ticket #10095 in combination with Track Ticket #10763). During development I came across the following two questions:

1. How should the code (mainly methods and members) be documented? I saw some files using doxygen style with \brief, \param and \return. Is this the prefered way to do it? And do you use some kind of groups with \ingroup or something like that?

2. I'm working on using a Service Mapping Description (SMD) to describe all the methods in XBMCs JSON RPC api but that requires quite some text. Until now I copied the text into the code (simply to be able to test if it works at all) but the SMD gets bigger and bigger and has reached a size of a few ten kBs (uncompressed).
So I have to get away from the "copy into the code" hack and need a way to read the SMD from a file. How are additional files handled by XBMC? It's not like some configuration file which can be changed by the user manually. If someone would change something in the SMD it would probably cause json rpc methods to fail. Where would such a file be stored and under which path could I access it in the code? Or is there a different approach for such files?

Thanks for your help.
Always read the online manual (wiki), FAQ (wiki) and search the forum before posting.
Do not e-mail Team Kodi members directly asking for support. Read/follow the forum rules (wiki).
Please read the pages on troubleshooting (wiki) and bug reporting (wiki) before reporting issues.
Reply

Logout Mark Read Team Forum Stats Members Help
The unofficial coding best practise and code formatting conventions for XBMC0