Revert previous commit, was incorrect
[amarok.git] / HACKING
blob260c9b8ed2fd7baa98d53a391b4bf5b2b4f17e03
1 Hacking on Amarok
2 -----------------
4 Please respect these guidelines when coding for Amarok, thanks!
6 * Where this document isn't clear, refer to Amarok code.
9 This C++ FAQ is a life saver in many situations, so you want to keep it handy:
11   http://www.parashift.com/c++-faq-lite/
14 Formatting
15 ----------
16 * Spaces, not tabs
17 * Indentation is 4 spaces
18 * Lines should be limited to 90 characters
19 * Spaces between brackets and argument functions
20 * For pointer and reference variable declarations put a space between the type
21   and the * or & and no space before the variable name.
22 * For if, else, while and similar statements put the brackets on the next line,
23   although brackets are not needed for single statements.
24 * Function and class definitions have their brackets on separate lines
25 * A function implementation's return type is on its own line.
26 * CamelCase.{cpp,h} style file names.
27 * Qt 4 includes a foreach keyword which makes it very easy to iterate over all
28   elements of a container.
30 Example:
32  | bool 
33  | MyClass::myMethod( QStringList list, const QString &name )
34  | {
35  |     if( list.isEmpty() )
36  |         return false;
37  |
38  |     /*
39  |       Define the temporary variable like this to restrict its scope
40  |       when you do not need it outside the loop. Let the compiler
41  |       optimise it.
42  |      */
43  |     foreach( QString string, list )
44  |     {
45  |
46  |         debug() << "Current string is " << string << endl;
47  |     }
48  | }
50 Header includes should be listed in the following order:
51     - Amarok includes
52     - KDE includes
53     - Qt includes
55 They should also be sorted alphabetically, for ease of locating them.  A small comment
56 if applicable is also helpful.
58 Includes in a header file should be kept to the absolute minimum, as to keep compile times
59 low. This can be achieved by using "forward declarations" instead of includes, like
60 "class QListView;" Forward declarations work for pointers and const references.
62 TIP:
63 Kate/KDevelop users can sort the headers automatically. Select the lines you want to sort,
64 then Tools -> Filter Selection Through Command -> "sort".
67 Example:
69  | #include "amarok.h"
70  | #include "debug.h"
71  | #include "playlist.h"
72  |
73  | #include <KDialogBase>    //baseclass
74  | #include <KPushButton>    //see function...
75  |
76  | #include <QGraphicsView>
77  | #include <QWidget>
80 Comments
81 --------
82 Comment your code. Don't comment what the code does, comment on the purpose of the code. It's
83 good for others reading your code, and ultimately it's good for you too.
85 Comments are essential when adding a strange hack, like the following example:
87  | /** Due to xine-lib, we have to make K3Process close all fds, otherwise we get "device is busy" messages
88  |   * Used by AmarokProcIO and AmarokProcess, exploiting commSetupDoneC(), a virtual method that
89  |   * happens to be called in the forked process
90  |   * See bug #103750 for more information.
91  |   */
92  | class AmarokProcIO : public K3ProcIO 
93  | {
94  |     public:
95  |     virtual int commSetupDoneC() {
96  |         const int i = K3ProcIO::commSetupDoneC();
97  |         Amarok::closeOpenFiles(K3ProcIO::out[0],K3ProcIO::in[0],K3ProcIO::err[0]);
98  |         return i;
99  |     }
100  | };
103 For headers, use the Doxygen syntax. See: http://www.stack.nl/~dimitri/doxygen/
105 Example:
107  | /**
108  |  * Start playback.
109  |  * @param offset Start playing at @p msec position.
110  |  * @return True for success.
111  |  */
112  | virtual bool play( uint offset = 0 ) = 0;
115 Header Formatting
116 -----------------
117 General rules apply here.  Please keep header function definitions aligned nicely,
118 if possible.  It helps greatly when looking through the code.  Sorted methods,
119 either by name or by their function (ie, group all related methods together) is
120 great too.
123  | #ifndef AMAROK_QUEUEMANAGER_H
124  | #define AMAROK_QUEUEMANAGER_H
126  | class QueueList : public K3ListView
127  | {
128  |         Q_OBJECT
130  |     public:
131  |         Queuelist( QWidget *parent, const char *name = 0 );
132  |         ~QueueList() {};
134  |     public slots:
135  |         void    moveSelectedUp();
136  |         void    moveSelectedDown();
137  | };
139 #endif /* AMAROK_QUEUEMANAGER_H */
142 0 vs NULL
143 ---------
144 The use of 0 to express a null pointer is preferred over the use of NULL.
145 0 is not a magic value, it's the defined value of the null pointer in C++.
146 NULL, on the other hand, is a preprocessor directive (#define) and not only is
147 it more typing than '0' but preprocessor directives are less elegant.
149  |     SomeClass *instance = 0;
152 Const Correctness
153 -----------------
154 Try to keep your code const correct. Declare methods const if they don't mutate the object,
155 and use const variables. It improves safety, and also makes it easier to understand the code.
157 See: http://www.parashift.com/c++-faq-lite/const-correctness.html
160 Example:
162  | bool 
163  | MyClass::isValidFile( const QString& path ) const
164  | {
165  |     const bool valid = QFile::exist( path );
167  |     return valid;
168  | }
171 Debugging
172 ---------
173 debug.h contains some handy functions for our debug console output.
174 Please use them instead of kDebug().
176 Usage:
178  | #include "debug.h"
180  | debug()   << "Something is happening" << endl;
181  | warning() << "Something bad may happen" << endl;
182  | error()   << "Something bad did happen!" << endl;
184 Additionally, there are some macros for debugging functions:
186 DEBUG_BLOCK
187 DEBUG_FUNC_INFO
188 DEBUG_LINE_INFO
189 DEBUG_INDENT
190 DEBUG_UNINDENT
192 AMAROK_NOTIMPLEMENTED
193 AMAROK_DEPRECATED
195 threadweaver.h has two additional macros:
196 DEBUG_THREAD_FUNC_INFO outputs the memory address of the current QThread or 'none'
197     if its the original GUI thread.
198 SHOULD_BE_GUI outputs a warning message if it occurs in a thread that isn't in
199     the original "GUI Thread", otherwise it is silent. Useful for documenting
200     functions and to prevent problems in the future.
203 Usage of Amarok::config()
204 -------------------------
205 We provide this method for convenience, but it is important to use it properly. By
206 inspection, we can see that we may produce very obscure bugs in the wrong case:
208  | KConfig 
209  | *config( const QString &group )
210  | {
211  |    //Slightly more useful config() that allows setting the group simultaneously
212  |    KGlobal::config()->setGroup( group );
213  |    return KGlobal::config();
214  | }
216 Take the following example:
218  | void
219  | f1()
220  | {
221  |    KConfig *config = Amarok::config( "Group 2" );
222  |    config->writeEntry( "Group 2 Variable", true );
223  | }
225  | void
226  | doStuff()
227  | {
228  |   KConfig *config = Amarok::config( "Group 1" );
229  |   f1();
230  |   config->writeEntry( "Group 1 Variable", true );
231  | }
233 We would expect the following results:
235  | [Group 1]
236  | Group 1 Variable = true
238  | [Group 2]
239  | Group 2 Variable = true
241 However because the config group is changed before writing the entry:
242  | [Group 1]
244  | [Group 2]
245  | Group 1 Variable = true
246  | Group 2 Variable = true
248 Which is clearly incorrect. And hard to see when your wondering why f1() is not
249 working. So do not store a value of Amarok::config, make it a habit to just 
250 always call writeEntry or readEntry directly.
252 Correct:
253  | amarok::config( "Group 1" )->writeEntry( "Group 1 Variable", true );
256 Errors & Asserts
257 ----------------
258 *Never use assert() or fatal(). There must be a better option than crashing a user's
259 application (its not uncommon for end-users to have debugging enabled).
261 *KMessageBox is fine to use to prompt the user, but do not use it to display errors
262 or informational messages. Instead, KDE::StatusBar has a few handy methods. Refer to
263 amarok/src/statusbar/statusBarBase.h
266 Commenting Out Code
267 -------------------
268 Don't keep commented out code. It just causes confusion and makes the source
269 harder to read. Remember, the last revision before your change is always
270 availabe in SVN. Hence no need for leaving cruft in the source.
272 Wrong:
273  | myWidget->show();
274  | //myWidget->rise(); //what is this good for?
276 Correct:
277  | myWidget->show();
280 Copyright
281 ---------
282 To comply with the GPL, add your name, email address & the year to the top of any file
283 that you edit. If you bring in code or files from elsewhere, make sure its
284 GPL-compatible and to put the authors name, email & copyright year to the top of
285 those files.
288 Thanks, now have fun!
289   -- the Amarok developers