and the binary classification is also not correct because only the ifrst component contains only one value
Description
Related Objects
- Mentioned In
- T27033: Clean up stale remote branches
Event Timeline
A style issue: please use always parentheses for if-blocks
In general I would prefer a solution that does not change the property but checks for the number of compenents at the location where the crash occurs.
I'd like to chime in here. :)
In my opinion the solution is totally okay. It is at the right place were our heuristic checks if an image is binary or not. There's already deliberate code to circumvent the check, i.e., it is checked if the binary property was already added. This property check is universal. It works both from code and UI.
Hence, +1 for the current solution. Not to mention that it is very clear at this place what the point of the author is even without commenting anything.
I'd like to see a little bit more code convention here as well, especially two spaces for indention and blank lines before and after the if-block. In that case I would even vote against parentheses. AFAIK we did never have this requirement in our style convention and it is uses widely in MITK. This restriction was only for if-else-blocks. :)
If everyone agrees I would vote for the Core Modification flag (given that the code style is improved).
Ok, having a closer look at _where_ the binary property is set in general, it's obviously not the clearest design and we could initialize it anywhere ... But where does the crash happen? There is always the possibility that someone changes the value of the binary property later (maybe even the user) and then it should not crash in my opinion.
You're right, I agree. The current solution does not prevent someone from changing the property later on. I didn't think about that. Where does the crash happen, Peter?
The crash happens in vtk but the last involved mit class is the mitk::VtkMapper in MitkRenderTranslucentGeometry line 87.
[df59e3]: Merge branch 'bug-18667-MultiComponentBinaryCrash2'
Merged commits:
2015-03-11 15:14:42 Peter Neher [1f5ecc]
setting Image.Displayed Component property correctly