Proposals:Refactoring Statistics Framework 2007 Transition Plan: Difference between revisions

From KitwarePublic
Jump to navigationJump to search
No edit summary
 
(8 intermediate revisions by the same user not shown)
Line 1: Line 1:
__TOC__
= Option A =
= Option A =


Line 19: Line 21:
= Option B =
= Option B =


- Keep all files in a single directory but use two different namespaces.
Keep all files in a single directory but use two different namespaces
 
== Method 1 ==


For instance the file itkSample.h would look like :
For instance the file itkSample.h would look like :


namespace itk
  namespace itk
   {
   {
   namespace StatisticsDeprecated
   namespace StatisticsDeprecated
Line 29: Line 33:
     class Sample : public Object { ... }
     class Sample : public Object { ... }
     }
     }
   namespace StatisticsNew
   namespace StatisticsNew
     {
     {
Line 38: Line 41:
- The ITK_USE_DEPRECATED_STATITSTICS_FRAMEWORK option redefines the unused namespace to be an anonymous namespace. The used namespace to "Statistics". For instance :
- The ITK_USE_DEPRECATED_STATITSTICS_FRAMEWORK option redefines the unused namespace to be an anonymous namespace. The used namespace to "Statistics". For instance :


  #ifdef ITK_USE_DEPRECATED_STATITSTICS_FRAMEWORK
    #ifdef ITK_USE_DEPRECATED_STATITSTICS_FRAMEWORK
    #define StatisticsDeprecated Statistics
      #define StatisticsDeprecated Statisticshow to put code in a wiki
     #define StatisticsNew
      #define StatisticsNew
     #else
      #define StatisticsNew Statistics
      #define StatisticsDeprecated
    #endif
 
User code still looks the same :
 
  #ifdef ITK_USE_DEPRECATED_STATISTICS_FRAMEWORK
    calculator = Statistics::CovarianceCalculator< ListSampleType >::New();
    calculator->SetInputSample(sample);
    calculator->Update();
   #else
   #else
     #define StatisticsNew Statistics
     filter = Statistics::CovarianceFilter< ListSampleType >::New();
     #define StatisticsDeprecated
     filter->SetInput( sampleGenerator->GetOutput() );
    filter->Update();
   #endif
   #endif


User code still looks the same :


#ifdef ITK_USE_DEPRECATED_STATISTICS_FRAMEWORK
== Method 2 ==
  calculator = Statistics::CovarianceCalculator< ListSampleType >::New();
 
   calculator->SetInputSample(sample);
I am not sure how you will be using an anonymous name space such as the following with what you suggested:
  calculator->Update();
 
#else
  namespace
  filter = Statistics::CovarianceFilter< ListSampleType >::New();
  {
  filter->SetInput( sampleGenerator->GetOutput() );
  }
  filter->Update();
 
#endif
I would think you would just need something similar to:
 
  namespace itk
  {
    namespace Statistics
    {
  #ifdef ITK_USE_DEPRECATED_STATISTICS_FRAMEWORK
    using namespace ::itk::StatisticsDeprecated;
  #else
    using namespace ::itk::StatisticsNew;
  #endif
    }
  }
 
= Option C =
 
Have two directories with separate namespaces.
 
Leave Statistics with its current namespace and do something like StatisticsV2 for the new one. It wouldn't surprise me it there were another statistics implementation in the future. After all, we are on our 4th DICOM.
 
= Option D =
 
* Take advantage of ITK 4.0 to integrate the New Statistics framework as is
 
The advantages over A and B
 
# no code duplication at all
# no surprise
# no additional cmake option, with all the advantages discussed here some time ago, including
## cleaner code
## easier to test
## can't confuse the user
 
And of course the disadvantage - can't see any other one
 
* break backward compatibility - should be acceptable if done not so often and comes with many enhancements for the user
 
 
Gaëtan
 
= Discussion =
 
== Option A versus Option B ==
 
=== Code duplication: ===
 
With A, if a file remains unchanged, it has to be duplicated (and maintained) in both directories.
 
With B, we have to worry only about the classes which need refactoring. Even refactoried files, where changes are minimal, you can even code of the form:
      namespace Statistics{    
        class Blah {
          #ifdef ITK_USE_DEPRECATED_STAT
              void SetInputSample( ListSampleType * );
          #else
              void SetInput( ListSampleType * );
          #endif
        }
      }
 
=== Code separation and readability ===
 
Option A separates the classes clearly in two directories. The statistics library will be devoid of ugly ifdefs. If we decide to drop the deprecated version at some point, we simply have to terminate a directory.
 
We can do the same with Option B, in a less obvious way, by separating the classes in two files, for instance :
   
      File StatistiitkSample.h:
        #include "StatisticsDeprecated/itkSample.h"
        #include "StatisticsNew/itkSample.h"
       
      File StatisticsDeprecated/itkSample.h
        namespace StatisticsDeprecated
          {
          class Sample : public Object { ... }
          }
 
      File StatisticsNew/itkSample.h
        namespace StatisticsNew
          {
          class Sample : public DataObject { ... }
          }
 
 
=== Surprises ===
 
* With Option A there are no surprises.
* I suspect that with B, using anonymized namespaces etc, in several translation units, some compiler might scream ? I tried to verify it with minimal C++ code on gcc4.3, but I feel uneasy about it.

Latest revision as of 11:39, 5 April 2009

Option A

  1. Rename the existing Statistics/ directory in ITK to StatisticsDeprecated/
  2. Create a parallel directory Statistics/ containing the new classes.
  3. Add an ITK_USE_DEPRECATED_STATITSTICS_FRAMEWORK option at configure time to toggle between the usage of the deprecated and the new framework. The option will cause (a) the right directory to be compiled (b) headers from there right directory to be installed (c) the right directory to be added to the include dirs.
  4. All existing code in the toolkit using the statistics framework will be ifdef'ed to work with both the new and the old framework. For instance :


 #ifdef ITK_USE_DEPRECATED_STATISTICS_FRAMEWORK
   calculator = Statistics::CovarianceCalculator< ListSampleType >::New();
   calculator->SetInputSample(sample);
   calculator->Update();
 #else
   filter = Statistics::CovarianceFilter< ListSampleType >::New();
   filter->SetInput( sampleGenerator->GetOutput() );
   filter->Update();
 #endif

Option B

Keep all files in a single directory but use two different namespaces

Method 1

For instance the file itkSample.h would look like :

 namespace itk
 {
 namespace StatisticsDeprecated
   {
   class Sample : public Object { ... }
   }
 namespace StatisticsNew
   {
   class Sample : public DataObject { ... }
   }
 }

- The ITK_USE_DEPRECATED_STATITSTICS_FRAMEWORK option redefines the unused namespace to be an anonymous namespace. The used namespace to "Statistics". For instance :

   #ifdef ITK_USE_DEPRECATED_STATITSTICS_FRAMEWORK
     #define StatisticsDeprecated Statisticshow to put code in a wiki 
     #define StatisticsNew
   #else
     #define StatisticsNew Statistics
     #define StatisticsDeprecated
   #endif

User code still looks the same :

 #ifdef ITK_USE_DEPRECATED_STATISTICS_FRAMEWORK
   calculator = Statistics::CovarianceCalculator< ListSampleType >::New();
   calculator->SetInputSample(sample);
   calculator->Update();
 #else
   filter = Statistics::CovarianceFilter< ListSampleType >::New();
   filter->SetInput( sampleGenerator->GetOutput() );
   filter->Update();
 #endif


Method 2

I am not sure how you will be using an anonymous name space such as the following with what you suggested:

 namespace 
 {
 }

I would think you would just need something similar to:

  namespace itk
  {
    namespace Statistics
    {
  #ifdef ITK_USE_DEPRECATED_STATISTICS_FRAMEWORK
    using namespace ::itk::StatisticsDeprecated; 
  #else
    using namespace ::itk::StatisticsNew;
  #endif
    }
  }

Option C

Have two directories with separate namespaces.

Leave Statistics with its current namespace and do something like StatisticsV2 for the new one. It wouldn't surprise me it there were another statistics implementation in the future. After all, we are on our 4th DICOM.

Option D

  • Take advantage of ITK 4.0 to integrate the New Statistics framework as is

The advantages over A and B

  1. no code duplication at all
  2. no surprise
  3. no additional cmake option, with all the advantages discussed here some time ago, including
    1. cleaner code
    2. easier to test
    3. can't confuse the user

And of course the disadvantage - can't see any other one

  • break backward compatibility - should be acceptable if done not so often and comes with many enhancements for the user


Gaëtan

Discussion

Option A versus Option B

Code duplication:

With A, if a file remains unchanged, it has to be duplicated (and maintained) in both directories.

With B, we have to worry only about the classes which need refactoring. Even refactoried files, where changes are minimal, you can even code of the form:

     namespace Statistics{   
       class Blah {
          #ifdef ITK_USE_DEPRECATED_STAT
             void SetInputSample( ListSampleType * );
          #else
             void SetInput( ListSampleType * );
          #endif
        }
      }

Code separation and readability

Option A separates the classes clearly in two directories. The statistics library will be devoid of ugly ifdefs. If we decide to drop the deprecated version at some point, we simply have to terminate a directory.

We can do the same with Option B, in a less obvious way, by separating the classes in two files, for instance :

     File StatistiitkSample.h:
        #include "StatisticsDeprecated/itkSample.h"
        #include "StatisticsNew/itkSample.h"
       
     File StatisticsDeprecated/itkSample.h
        namespace StatisticsDeprecated
         {
         class Sample : public Object { ... }
         }
     File StatisticsNew/itkSample.h
        namespace StatisticsNew
         {
         class Sample : public DataObject { ... }
         }


Surprises

  • With Option A there are no surprises.
  • I suspect that with B, using anonymized namespaces etc, in several translation units, some compiler might scream ? I tried to verify it with minimal C++ code on gcc4.3, but I feel uneasy about it.