From: frog Date: Thu, 24 Jun 2004 11:44:35 +0000 (+0000) Subject: * DEVELOPPER: added a proposition of coding style. X-Git-Tag: Version0.5.bp~101 X-Git-Url: https://git.creatis.insa-lyon.fr/pubgit/?a=commitdiff_plain;h=d71e2e62b38383d9b960760f6d81c4d545d59fba;p=gdcm.git * DEVELOPPER: added a proposition of coding style. * src/gdcmDocEntry.h: removed every inline declaration (for test of coding style). --- Frog --- diff --git a/ChangeLog b/ChangeLog index 52c3537b..4a9d4786 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +2004-06-23 Eric Boix + * DEVELOPPER: added a proposition of coding style. + * src/gdcmDocEntry.h: removed every inline declaration (for test of + coding style). + 2004-06-23 Eric Boix * gdcmDocEntry::PrintCommonPart() and ::WriteCommonPart() removed. Use the gdcmDocEntry::Print() and Write() instead. diff --git a/DEVELOPPER b/DEVELOPPER index f70ac33a..3f01c29e 100644 --- a/DEVELOPPER +++ b/DEVELOPPER @@ -103,3 +103,263 @@ The following comments are intended for core gdcm developpers. 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 + instead of #include ). + - 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 +------------------------------------------------ +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 +------------------------------------------------ diff --git a/src/gdcmDocEntry.h b/src/gdcmDocEntry.h index 9eafc568..4ee6f7f5 100644 --- a/src/gdcmDocEntry.h +++ b/src/gdcmDocEntry.h @@ -3,8 +3,8 @@ Program: gdcm Module: $RCSfile: gdcmDocEntry.h,v $ Language: C++ - Date: $Date: 2004/06/23 13:02:36 $ - Version: $Revision: 1.11 $ + Date: $Date: 2004/06/24 11:44:35 $ + Version: $Revision: 1.12 $ Copyright (c) CREATIS (Centre de Recherche et d'Applications en Traitement de l'Image). All rights reserved. See Doc/License.txt or @@ -40,76 +40,75 @@ public: gdcmDocEntry(gdcmDictEntry*); /// Returns the Dicom Group number of the current Dicom Header Entry - inline guint16 GetGroup(void) { return entry->GetGroup(); }; + guint16 GetGroup(void) { return entry->GetGroup(); }; /// Returns the Dicom Element number of the current Dicom Header Entry - inline guint16 GetElement(void) { return entry->GetElement();}; + guint16 GetElement(void) { return entry->GetElement();}; /// Returns the 'key' of the current Dicom Header Entry - inline std::string GetKey(void) { return entry->GetKey(); }; + std::string GetKey(void) { return entry->GetKey(); }; /// \brief Returns the 'Name' '(e.g. "Patient's Name") found in the Dicom /// Dictionnary of the current Dicom Header Entry - inline std::string GetName(void) { return entry->GetName(); }; + std::string GetName(void) { return entry->GetName(); }; /// \brief Returns the 'Value Representation' (e.g. "PN" : Person Name, /// "SL" : Signed Long), found in the Dicom Header or in the Dicom /// Dictionnary, of the current Dicom Header Entry - inline std::string GetVR(void) { return entry->GetVR(); }; + std::string GetVR(void) { return entry->GetVR(); }; /// \brief Returns offset (since the beginning of the file, including /// the File Pramble, if any) of the value of the current Dicom HeaderEntry /// \warning offset of the *value*, not of the Dicom Header Entry - inline size_t GetOffset(void) { return Offset; }; + size_t GetOffset(void) { return Offset; }; /// \brief Returns the actual value length of the current Dicom Header Entry /// \warning this value is not *always* the one stored in the Dicom Header /// in case of well knowned bugs - inline guint32 GetLength(void) { return UsableLength; }; + guint32 GetLength(void) { return UsableLength; }; /// \brief Returns the 'read length' of the current Dicom Header Entry /// \warning this value is the one stored in the Dicom Header but not /// mandatoryly the one thats's used (in case on SQ, or delimiters, /// the usable length is set to zero) - inline guint32 GetReadLength(void) { return ReadLength; }; + guint32 GetReadLength(void) { return ReadLength; }; /// Sets the 'Value Representation' of the current Dicom Header Entry - inline void SetVR(std::string v) { entry->SetVR(v); }; + void SetVR(std::string v) { entry->SetVR(v); }; /// \brief Sets both 'Read Length' and 'Usable Length' of the current /// Dicom Header Entry - inline void SetLength(guint32 l) { ReadLength=UsableLength=l;}; + void SetLength(guint32 l) { ReadLength=UsableLength=l;}; // The following 3 members, for internal use only ! /// \brief Sets only 'Read Length' (*not* 'Usable Length') of the current /// Dicom Header Entry - inline void SetReadLength(guint32 l) { ReadLength = l; }; + void SetReadLength(guint32 l) { ReadLength = l; }; /// \brief Sets only 'Usable Length' (*not* 'Read Length') of the current /// Dicom Header Entry - inline void SetUsableLength(guint32 l) { UsableLength = l; }; + void SetUsableLength(guint32 l) { UsableLength = l; }; /// \brief Sets the offset of the Dicom Element /// \warning use with caution ! /// @param of offset to be set - inline void gdcmDocEntry::SetOffset(size_t of) { Offset = of; }; + void gdcmDocEntry::SetOffset(size_t of) { Offset = of; }; /// Sets to TRUE the ImplicitVr flag of the current Dicom Element - inline void gdcmDocEntry::SetImplicitVR(void) { ImplicitVR = true; }; + void gdcmDocEntry::SetImplicitVR(void) { ImplicitVR = true; }; /// \brief Tells us if the current Dicom Element was checked as ImplicitVr /// @return true if the current Dicom Element was checked as ImplicitVr - inline bool gdcmDocEntry::IsImplicitVR(void) { return ImplicitVR; }; + bool gdcmDocEntry::IsImplicitVR(void) { return ImplicitVR; }; /// \brief Tells us if the VR of the current Dicom Element is Unknown /// @return true if the VR is unkonwn - inline bool gdcmDocEntry::IsVRUnknown(void) - { return entry->IsVRUnknown(); }; + bool gdcmDocEntry::IsVRUnknown(void) { return entry->IsVRUnknown(); }; /// \brief Sets the DicEntry of the current Dicom Element /// @param NewEntry pointer to the DictEntry - inline void gdcmDocEntry::SetDictEntry(gdcmDictEntry *NewEntry) + void gdcmDocEntry::SetDictEntry(gdcmDictEntry *NewEntry) { entry = NewEntry; }; /// \brief Gets the DicEntry of the current Dicom Element @@ -135,11 +134,11 @@ public: /// \brief Gets the depth level of a Dicom header entry embedded in /// a SeQuence - inline int GetDepthLevel(void) {return(SQDepthLevel);} + int GetDepthLevel(void) {return(SQDepthLevel);} /// \brief Sets the depth level of a Dicom header entry embedded in /// a SeQuence - inline void SetDepthLevel(int depth) {SQDepthLevel = depth;} + void SetDepthLevel(int depth) {SQDepthLevel = depth;} private: // FIXME: In fact we should be more specific and use :