]> Creatis software - gdcm.git/blobdiff - DEVELOPPER
* DEVELOPPER: added a proposition of coding style.
[gdcm.git] / DEVELOPPER
index f4fb12ff45b6886d52560fb33e77dbe0567eac3b..3f01c29ec3bcf2148c890f6e42edba9ad36d1156 100644 (file)
 The following comments are intended for core gdcm developpers.
 
+* Compiling gdcm:
+  - Checkout the sources to an arbitrary directory noted GDCM (e.g. ~/cvs/gdcm).
+  - Checkout the test images to an arbitrary directory noted GDCMDATA
+    (e.g. ~/cvs/gdcm/gdcmData).
+  - Optionally create a temporary installation directory GDCMINSTALL e.g.
+    mkdir /tmp/gdcminstall
+  - Create a target directory GDCMBIN e.g.
+    mkdir /tmp/gdcmbin
+  - Configure cmake from GDCMBIN:
+    cd GDCMBIN
+    ccmake GDCM
+      * Toggle and adjust the required options and parameters, mainly
+        -- GDCM_DATA_ROOT should be set to GDCMDATA
+        -- CMAKE_INSTALL_PREFIX (equivalent of --prefix of the autotools)
+           should be set to GDCMINSTALL
+        -- CMAKE_BUILD_TYPE set to Debug or Release
+        -- GDCM_DOXYGEN 
+        -- GDCM_VTK When this option is on VTK_DIR might require manual
+           configuration
+        -- GDCM_WRAP_PYTHON
+      * Configure cmake: hit c
+      * Generate the makefiles (or dsw): hit g
+  - Compile gdcm
+    make  
+  - Run the tests manually (optional):
+    Caveat: if you run the tests before installing, you NEED to positionate
+            the environment variable GDCM_DICT_PATH to GDCM/Dicts e.g.
+            export GDCM_DICT_PATH=~/cvs/gdcm/Dicts
+    Assuming your cwd is GDCMBIN, three modes are then available:
+    -- 1/ acces to a by number:
+          ./Test/gdcmCxxTests
+    -- 2/ acces to a test by it's name, by passing it as argument to
+          gdcmCxxTests e.g.
+          ./Test/gdcmCxxTests hashtest
+    -- 3/ launch all the full test suite (again we assume cwd is GDCMBIN):
+          ctest
+          This is equivalent to "make test".
+          Note: ctest supports the argument filtering with regexp and
+                the verbose mode e.g.
+                   ctest -R print -V
+                proposes the tests containing "print" in their name and
+                makes a verbose output. For other options refer to the
+                documentation at http://www.cmake.org.
+
+* Sending the result to kitware's dashboard (optional)
+  ctest -D Experimental
+  The results should appear in
+     http://public.kitware.com/dashboard.php?name=public
+     under the name of your machine (uname), but for ease of use you can
+     change the SITE variable in your CMakeCache.txt to something more 
+     accurate such as: GDCM-my_machine_name. The entry will then be within 
+     the "Experimental Builds" entry.
+
+* Install gdcm (as specified by CMAKE_INSTALL_PREFIX)
+  make install
+  Note: the dictionaries used by gdcm are now located in
+        CMAKE_INSTALL_PREFIX + /share/ (i.e. GDCMINSTALL + /share/ if you
+        followed the above instructions).
 
 * Python related section.
-  Depending on the automake/autoconf/autogen.sh flags you used gdcm could
+  Depending on the cmake flags you used in order to compile gdcm could
   be wrapped in two ways:
-  - the first python wrappers of gdcm use Swig (http://www.swig.org). They
-    are the ones generated when using autogen.sh --enable-python. 
-    The entry point is here the file gdcmPython/gdcm.i which uses the
-    Swig synthax. As the last lines of this file (the ones starting
+  - the first python wrappers of gdcm uses Swig (http://www.swig.org). These
+    are the one generated when using autogen.sh --enable-python. 
+    The entry point here is the file gdcmPython/gdcm.i which uses the
+    Swig syntax. As the last lines of this file (the ones starting
     with the %include directive) only some classes are wrapped for python.
     In theory only the library interface (basically the classes gdcmHeader
     and gdcmFile) should be wrapped, but the time being some additional
     classes are added (just to make sure those classes are Swig compatible:
-    swig is here used as some lint checker!?).
-    Since gdcm is written in C++, Swig will produce two different outups:
+    swig is here used as some link checker!?).
+    Since gdcm is written in C++, Swig will produce two different outputs:
       -- some C based low level wrapper (see gdcmPython/gdcm_wrap.c)
       -- some Python based object oriented so called "shadow classes" (see
          file gdcmPython/gdcm.py)
     We also added the file gdcmPython/__init__.py which is the one that
-    actually gets loaded when ones uses the gdcmPython Python package.
-    The file __init__.py loads the swig generated shadow classes (gdcm.py)
-    but will only re-export the interface of gdcm which correponds to
+    actually gets loaded when one uses the gdcmPython Python package.
+    The file __init__.py loads the Swig generated shadow classes (gdcm.py)
+    but will only re-export the interface of gdcm which corresponds to
     the lines :
        gdcmHeader = gdcm.gdcmHeader
        gdcmDictSet = gdcm.gdcmDictSet
        gdcmFile = gdcm.gdcmFile
+       [etc.]
     Hence this whole Swig wrapping process is quite odd since we shall
-    wrap more classes (%include in swig.i) than eventually exported to
+    wrap more classes (%include in swig.i) that eventually get exported to
     the final user by gdcmPython/__init__.py.
-  - the second python wrappers use the vtk (http://public.kitware.com/VTK/)
+  - the second python wrappers uses the vtk (http://public.kitware.com/VTK/)
     native wrappers i.e. the binary vtkWrapPython. But it should be noticed
-    that the purpous is here a bit different than the one of the Swig
+    that the purpose is here a bit different than the one of the Swig
     generated Python wrappers. When using vtkWrapPython the goal is to
     wrap a single vtk class namely vtkGdcmReader as defined in files
-    in vtk/vtkGdcmReader.h vtk/vtkGdcmReader.cxx (and of course those
+    vtk/vtkGdcmReader.h and vtk/vtkGdcmReader.cxx (and of course those
     files are hand made vtk oriented wrappers of gdcm).
     Those wrappers are the one generated when using
        autogen.sh --enable-python --enable-vtk
   - In order to understand the difference between both wrappers you should 
     compare both demo scripts gdcmPython/demo/vtkGdcmDemo.py and
     gdcmPython/demo/vtkGdcmReader.py. The first one only uses the
-    Swig wraped classes ("from gdcmPython import gdcmHeader') as opposed
-    to vtkGdcmReader.py which also uses vtkWrapPython wraped classes
+    Swig wrapped classes ("from gdcmPython import gdcmHeader') as opposed
+    to vtkGdcmReader.py which also uses vtkWrapPython wrapped classes
     ("from gdcmPython.vtkgdcmPython import *").
     
+------------------------------------------------------------------------------
+gdcm coding style and religious/agnostic beliefs (proposition)
+   
+* Introduction:
+   The following coding style intends to ease the work of developpers
+   themselves but also of users who will study, maintain, fix, and extend
+   the code. Any bread crumbs that you can drop in the way of explanatory
+   names and comments will go a long way towards helping other readers and
+   developers.
+   Keep in mind that to a large extent the structure of code directly
+   expresses its implementation.
+
+* Language:
+ - C++ (for the kernel) and Python (for the wrappers).
+ - all the names (variables, members, methods, functions) and comments
+   should be based on English. Documentation, guides, web site and other
+   informations should be in English.
+   Make sure you use correct (basic) English and complete, grammatically
+   correct sentences for comments and documentation.
+
+* General layout:
+ - Each line of code should take no more than 79 characters. Break the code
+   across multiple lines as necessary. 
+ - Methods and functions should keep a reasonable number of lines when
+   possible (a typical editor displays 50 lines). Avoid code duplication.
+   Allways prefer creating a new method or function to duplication.
+   A high indentation level generally suggests the need for a new
+   method or function.
+ - All the code should be properly indented. The appropriate indentation
+   level is three spaces for each level of indentation. DO NOT USE TABS.
+   Set up your editor to insert spaces. Using tabs may look good in your
+   editor but will wreak havoc in others, or in external tools (e.g. side
+   by side diffs).
+ - The declaration of variables within classes, methods, and functions
+   should be one declaration per line. Provide them with default values
+   and don't rely on compilers for default initialization.
+
+* Naming conventions:
+ - Generalities:
+   In general, names are constructued by using case change to indicate
+   separate words, as in ImageDataSize (standing for "image data size").
+   Underscores are not used. Variable names are chosen carefully with the
+   intention to convey the meaning behind the code. Names are generally
+   spelled out; use of abbreviations is discouraged.
+   [Note: abbreviation are allowable when in common use, and should be in
+    uppercase as in LUT or RGBA.]
+   While this does result in long names, it self-documents the code.
+ - Naming Files:
+   Files should have the same name as the class, with an "gdcm" prepended.
+   Header files are named .h, while implementation files are named either
+   .cxx or .txx, depending on whether they are implementations of templated
+   classes. For example, the class gdcm::Document is declared and defined
+   in the files gdcmDocument.h and gdcmDocument.cxx.
+ - Naming Class Data Members, Methods, and Functions:
+   Class data members are named beginning with a capital letter as in
+   GroupPixel.
+   Global functions and class methods, either static or class members, are
+   named beginning with a capital letter, as in GetImageDataSize().
+ - Naming Local Variables:
+   Local variables begin in lowercase. There is more flexibility in the
+   naming of local variables allthough they still should convey some
+   semantics.
+
+* Classes:
+ - Don't use the inline keyword when defining an inline function
+   within a class definition.
+ - Do not repeat the virtual keyword when overriding virtual base methods
+   in declaration of subclasses:
+     class A 
+     {
+        virtual void foo(...);
+     };
+     class B: public A
+     {
+        void foo(...);          // and not: virtual void foo(...);
+     };
+ - The public, protected, private declarations should be at the
+   same indent level as the class. Use
+     class A 
+     {
+     private:
+        void foo(...);
+     public:
+        void bar(...);
+     };
+ - Method and functions devoided of arguments should not use the void
+   notation. Use
+     SomeType Header::GetPixelData()
+   and not
+     SomeType Header::GetPixelData(void).
+
+* Use of braces:
+ - Braces must be used to delimit the scope of an if, for, while, switch, or
+   other control structure. Braces are placed on a line by themselves, and
+   at the same indentation level as the control structure to which they
+   belong:
+      for (i=0; * i<3; i++)
+      {
+         ...
+      }
+   or when using an if:
+      if ( condition )
+      {
+         ...
+      }
+      else if ( other condition )
+      {
+         ...
+      }
+      else
+      {
+         ....
+      }
+   You can choose to use braces on a line with a code block when
+   the block consists of a single line:
+      if ( condition ) { foo=1; }
+      else if ( condition2 ) { foo=3; }
+      else { return; }
+   or
+      for (i=0; i<3; ++i) {x[i]=0.0;}
+   Methods and functions should follow the same usage of braces:
+      void File::ParsePixelData()
+      {
+         ...
+      }
+
+* Special layout:
+ - Avoid code mixed with comments on a single line. Instead, prepend the
+   logical blocks of code with the concerned comments.
+ - Use parantheses around conditions e.g. with an if statement: 
+      if ( someLocalVariable == 2 ) { ... }
+ - Add spaces around parantheses, or braces. Use
+      if ( someLocalVariable == 2 ) { ClassMenber += 1; }
+   and not
+      if (someLocalVariable == 2) {ClassMenber += 1;}
+ - Add spaces around each side of the assignement operator, and
+   around binary operators used in boolean expression. Use
+      someLocalVariable = ClassMember * 2;
+      if ( someLocalVariable == 2 || ClassMember == 2 ) ...
+   and not
+      someLocalVariable=ClassMember*2;
+      if ( someLocalVariable==2||ClassMember==2 ) ...
+
+* Miscelaneous:
+ - Don't use underscores. Don't use tabs. Don't use control characters
+   like ^M. Anyhow, cvs is configured to reject such commits.
+ - Comments should be in C++ style ("// ...", two slashes, per line). Don't
+   use C style comments ("/* ... */").
+ - The last line of a file should terminate with "\n".
+ - Returned arguments of methods and functions should not be wrapped with
+   parantheses. Use 
+      return iter->second;
+   but do not use
+      return ( iter->second );
+
+* Debugging and Verbose modes:
+   Never use std::cout. Instead use the gdcmDebug class through the
+   global instance dbg. Example:
+      #include "gdcmDebug.h"
+      ...
+      {
+         dbg.Verbose(3, "Local function name: entering.");
+         ...
+      }
+    will send the message to std::cout when dbg.Debug() is called
+    in your main.
+
+* Documentation:
+   The Doxygen open-source system is used to generate on-line documentation.
+   Doxygen requires the embedding of simple comments in the code which is in
+   turn extracted and formatted into documentation. See
+      http://www.stack.nl/ dimitri/doxygen/
+   for more information about Doxygen.
+ - Documenting a class:
+   Classes should be documented using the class and brief doxygen commands,
+   followed by the detailed class description:
+      /**
+       * \class Header
+       * \brief Header acts as container of Dicom elements of an image.
+       *
+       * Detailed description of the class is provided here
+       * ...
+       */
+   The key here is that the comment starts with /**, each subsequent line has
+   an aligned *, and the comment block terminates with a */.
+ - Documenting class members and inline methods:
+   All the members and the inline methods should be documented within
+   the class declaration as shown in the following example:
+      class Header
+      {
+         /// True when parsing was succefull. False otherwise.
+         bool Readable = false;
+
+         /// \brief The number of lines of the image as interpreted from
+         /// the various elements encountered at header parsing.
+         int NumberOfLines = -1;
+
+         /// Predicate implemented as accessor around \ref Readable.
+         bool IsReadable() { return Readable; }
+      };
+ - Documenting a Method:
+   Methods should be documented using the following comment block style
+   as shown in the following example:
+
+      /**
+       * \brief  Within the Dicom Elements (parsed with the public and private
+       *         dictionaries), look for the element value representation of
+       *         a given tag.
+       * @param  group Group number of the searched tag.
+       * @param  element Element number of the searched tag.
+       * @return Corresponding element value representation when it exists,
+       *         and the string "gdcm::Unfound" otherwise.
+       */
+      std::string Document::GetEntryByNumber(guint16 group, guint16 element)
+      {
+         ...
+      }
+
+* External includes and C style:
+ - Only the C++ standard library and the STL includes should be used.
+   When including don't use the .h extension (use #include <iostream>
+   instead of #include <iostream.h>).
+ - Don't use the C standard library. Don't include stdio.h, ctype.h...
+   Don't use printf(), sprinf(), FILE*...
+ - Don't use the NULL notation (either as macro, or as const int NULL=0).
+   A pointer that doesn't refer to an object should simply be defined as
+      DataPointer* MyDataPointer = 0;
+
+* Basic types:
+ - Assume T is a given type. When declaring or defining with the 
+   "pointer to T" notation, the * character must be adjacent to
+   the type and not the variable. That is use
+      T* foo = 0;
+   and not 
+      T *foo = 0;
+ - Allways define a typedef for a new type and be consistent in usage.
+   Use
+      typedef Header* HeaderPointer;
+      HeaderPointer MyHeaderPointer;
+ - One notorious counter example for non using C style inclusion concerns
+   exact-width intergers (since there seem to be no equivalent for C++).
+   When using exact-width integers use the typedef names defined by
+   the Basic ISO C99: 7.18 Integer types i.e.
+      int8_t     int16_t     int32_t     int64_t (signed integers)
+   and
+      uint8_t    uint16_t    uint32_t    uint64_t (unsigned integers).
+   Hence do not use declarations like "unsigned int".
+   With g++, accessing those typedef is achieved by the following
+      #include <stdint.h>
+------------------------------------------------
+CVS policy
+* All the commits should be atomic. They must preserve the compilation
+  in order to prevent checkouts with broken code.
+* All the commits must correspond to a stade of the code where ctest
+  runs and has no failing subtest. Allways run ctest before commiting.
+  Note: * you can launch ctest in verbose mode through the command
+             ctest -V >& log
+        * you can launch a single test through ctest with
+             ctest -R FailingTestName -V >& log
+------------------------------------------------