(0011862)
Luis Ibanez (manager)
2008-05-13 12:01

Unfortunately we can not use partial specialization because it is not supported in Visual Studio 6.0 and we still support this compiler.

(0011863)
Luis Ibanez (manager)
2008-05-13 12:02

How about replacing instances of typeid with using "dynamic_cast" ?

(0011869)
Gert Wollny (reporter)
2008-05-13 13:35

* To cite Strounstrup: "Using dynamic_cast rather than typeid would improve code quality only marginally" - it would still be a run-time decision - and in the FFTW application cases it would not be possible because the typeid is applied to identify float and double pixel types.

* Still VC 6.0 support, seems you are better than MS. VC6 is now 10 years old, maybe one should ... hmm ... move on? I mean, one can download VC++ Express 2008 for free (haven't tried it, though).

(0011873)
Luis Ibanez (manager)
2008-05-13 15:40

We have requested feedback from the users list:

http://www.itk.org/pipermail/insight-users/2008-May/025838.html [^]

regarding how many users are still relying on Visual Studio 6.0.

(0016971)
Bill Lorensen (developer)
2009-07-26 11:06

I agree that partial specialization is a better method than run-time decisions based on typeid. However, not all of the supported ITK compilers support partial specialization.

(0016975)
Gert Wollny (reporter)
2009-07-27 04:14

Maybe that's something to be left for version 4.0: to redefine the set of supported compilers, make sure partial specialization is part of the required feature set, and then fix these kind f issues.

(0017019)
Bill Lorensen (developer)
2009-07-30 08:03

We will leave this issue open as a reminder of the limitations of some of our current compilers.

(0022826)
Bill Lorensen (developer)
2010-11-04 00:42

Now that we have dropped VS6,7 and Borland, we can address this issue.

(0022827)
Bill Lorensen (developer)
2010-11-04 00:44

CannySegmentationLevelSetFunction has been fixed. See this gerrit topic:
http://review.source.kitware.com/#change,240 [^]

(0022849)
Hans Johnson (developer)
2010-11-04 15:17

The patch can help get rid of many typeid constructs, and can now be applied now that partial specialization is allowed.

(0022900)
kentwilliams (developer)
2010-11-05 12:58

I'm readying a push to gerrit that more or less comprehensively addresses this idea of removing typeids.

Notes:

1. Gaetan has a separate rewrite of Morphology-related classes that removes a bunch of instances.
2. Many classes use typeid to get a type name string for debugging -- not fixed.
3. Some classes have long lists of typeids, that -- as in the ImageIO cases -- aren't easily replaced
4. typeid -> type name string is another common use.
5. ITK/Code/Review/itkFFTComplexToComplexImageFilter has some stylistic problems, notably no implementation unencumbered by FFTW. Fixing this would require more work than I can put into it in a reasonable timeframe.

(0022911)
Gert Wollny (reporter)
2010-11-05 14:04

some remarks:

2 - Using typeid to get a type name for debugging is not the issue, making run-time decisions based on typeid is, since type based decisions can be resolved at compile time and errors will be reported early. Also, run-time decisions add a performance penalty that may be of consequence.

3 - at least what i've seen in ImageIO looks like the perfect opportunity to use a pixel type trait class. It also makes it easier to add another type, because one only needs to define a new specialization of the trait together with the new type and one doesn't have to touch the long list of ifs.
Disadvantage: for this to function the code needs to be templated.

4 - same resolution as 3.

(0023499)
kentwilliams (developer)
2010-11-22 17:28

One fix has been pushed already, and I've added a new topic to address the remaining issues I could discover:
http://review.source.kitware.com/#change,424 [^]

There are typeids left but they're either a) using a typeid for printing debug information, or b) a run-time type check that can't be replaced with Template Metaprogramming.

Issue History
Date Modified	Username	Field	Change
2008-05-13 10:21	Gert Wollny	New Issue
2008-05-13 10:21	Gert Wollny	File Added: InsightToolkit-3.6.0-Code-Algorithms-remove-typeid.diff
2008-05-13 12:01	Luis Ibanez	Note Added: 0011862
2008-05-13 12:02	Luis Ibanez	Note Added: 0011863
2008-05-13 13:35	Gert Wollny	Note Added: 0011869
2008-05-13 15:40	Luis Ibanez	Note Added: 0011873
2009-07-26 11:04	Bill Lorensen	Status	new => assigned
2009-07-26 11:04	Bill Lorensen	Assigned To	=> Bill Lorensen
2009-07-26 11:06	Bill Lorensen	Note Added: 0016971
2009-07-26 11:06	Bill Lorensen	Status	assigned => feedback
2009-07-27 04:14	Gert Wollny	Note Added: 0016975
2009-07-30 08:03	Bill Lorensen	Note Added: 0017019
2009-07-30 08:04	Bill Lorensen	Status	feedback => acknowledged
2010-11-04 00:42	Bill Lorensen	Note Added: 0022826
2010-11-04 00:44	Bill Lorensen	Note Added: 0022827
2010-11-04 00:46	Bill Lorensen	Sprint Status	=> backlog
2010-11-04 00:46	Bill Lorensen	Resolution	open => suspended
2010-11-04 15:17	Hans Johnson	Status	acknowledged => assigned
2010-11-04 15:17	Hans Johnson	Assigned To	Bill Lorensen => kentwilliams
2010-11-04 15:17	Hans Johnson	Note Added: 0022849
2010-11-04 15:17	Hans Johnson	Resolution	suspended => reopened
2010-11-05 12:58	kentwilliams	Note Added: 0022900
2010-11-05 13:19	kentwilliams	Resolution Date	=> 2010-11-05
2010-11-05 13:19	kentwilliams	Sprint Status	backlog => active
2010-11-05 13:19	kentwilliams	Status	assigned => resolved
2010-11-05 13:19	kentwilliams	Product Version	=> ITK-4-A0
2010-11-05 14:04	Gert Wollny	Note Added: 0022911
2010-11-05 14:04	Gert Wollny	Status	resolved => feedback
2010-11-22 17:28	kentwilliams	Resolution Date	2010-11-05 => 2010-11-22
2010-11-22 17:28	kentwilliams	Sprint Status	active => completed
2010-11-22 17:28	kentwilliams	Note Added: 0023499
2010-11-22 17:28	kentwilliams	Status	feedback => closed
2010-11-22 17:28	kentwilliams	Resolution	reopened => fixed

Notes
(0011862) Luis Ibanez (manager) 2008-05-13 12:01	Unfortunately we can not use partial specialization because it is not supported in Visual Studio 6.0 and we still support this compiler.

(0011863) Luis Ibanez (manager) 2008-05-13 12:02	How about replacing instances of typeid with using "dynamic_cast" ?

(0011869) Gert Wollny (reporter) 2008-05-13 13:35	* To cite Strounstrup: "Using dynamic_cast rather than typeid would improve code quality only marginally" - it would still be a run-time decision - and in the FFTW application cases it would not be possible because the typeid is applied to identify float and double pixel types. * Still VC 6.0 support, seems you are better than MS. VC6 is now 10 years old, maybe one should ... hmm ... move on? I mean, one can download VC++ Express 2008 for free (haven't tried it, though).

(0011873) Luis Ibanez (manager) 2008-05-13 15:40	We have requested feedback from the users list: http://www.itk.org/pipermail/insight-users/2008-May/025838.html [^] regarding how many users are still relying on Visual Studio 6.0.

(0016971) Bill Lorensen (developer) 2009-07-26 11:06	I agree that partial specialization is a better method than run-time decisions based on typeid. However, not all of the supported ITK compilers support partial specialization.

(0016975) Gert Wollny (reporter) 2009-07-27 04:14	Maybe that's something to be left for version 4.0: to redefine the set of supported compilers, make sure partial specialization is part of the required feature set, and then fix these kind f issues.

(0017019) Bill Lorensen (developer) 2009-07-30 08:03	We will leave this issue open as a reminder of the limitations of some of our current compilers.

(0022826) Bill Lorensen (developer) 2010-11-04 00:42	Now that we have dropped VS6,7 and Borland, we can address this issue.

(0022827) Bill Lorensen (developer) 2010-11-04 00:44	CannySegmentationLevelSetFunction has been fixed. See this gerrit topic: http://review.source.kitware.com/#change,240 [^]

(0022849) Hans Johnson (developer) 2010-11-04 15:17	The patch can help get rid of many typeid constructs, and can now be applied now that partial specialization is allowed.

(0022900) kentwilliams (developer) 2010-11-05 12:58	I'm readying a push to gerrit that more or less comprehensively addresses this idea of removing typeids. Notes: 1. Gaetan has a separate rewrite of Morphology-related classes that removes a bunch of instances. 2. Many classes use typeid to get a type name string for debugging -- not fixed. 3. Some classes have long lists of typeids, that -- as in the ImageIO cases -- aren't easily replaced 4. typeid -> type name string is another common use. 5. ITK/Code/Review/itkFFTComplexToComplexImageFilter has some stylistic problems, notably no implementation unencumbered by FFTW. Fixing this would require more work than I can put into it in a reasonable timeframe.

(0022911) Gert Wollny (reporter) 2010-11-05 14:04	some remarks: 2 - Using typeid to get a type name for debugging is not the issue, making run-time decisions based on typeid is, since type based decisions can be resolved at compile time and errors will be reported early. Also, run-time decisions add a performance penalty that may be of consequence. 3 - at least what i've seen in ImageIO looks like the perfect opportunity to use a pixel type trait class. It also makes it easier to add another type, because one only needs to define a new specialization of the trait together with the new type and one doesn't have to touch the long list of ifs. Disadvantage: for this to function the code needs to be templated. 4 - same resolution as 3.

(0023499) kentwilliams (developer) 2010-11-22 17:28	One fix has been pushed already, and I've added a new topic to address the remaining issues I could discover: http://review.source.kitware.com/#change,424 [^] There are typeids left but they're either a) using a typeid for printing debug information, or b) a run-time type check that can't be replaced with Template Metaprogramming.

Relationships