Refactor a huge Switch Statement (1400 lines of Code)

12 Ansichten (letzte 30 Tage)
Julian
Julian am 19 Sep. 2018
Kommentiert: Julian am 19 Sep. 2018
Hi everyone,
so i have a programm and part of that programm is a specific image processing routine. I didn't write this myself but i have to refactor it and i want to know whether there is some kind of standard method to refactor switch statements.
I give you an example of the code:
case 'Use ROI'
if(2==exist(fs{1},'file'))
try
load(fs{1},'-mat');
BW = roipoly(tmpImg,xy(:,1),xy(:,2));
if(isnumeric(fs{2})&&fs{2}) % inverted
strImg.pro = ~BW.*tmpImg;
else
strImg.pro = BW.*tmpImg;
end
catch
warning('Could not load ROI');
strImg.pro = tmpImg;
end
else
strImg.pro = tmpImg;
warning(['Could not load ROI file ''',fs{1},'''']);
end
case 'Blurring'
h = fspecial('average',fs{1});
if(fs{1}>200)
pad = 0;%fs{1};
sub = .5*pad;
F = fft2(tmpImg, size(tmpImg,1)+pad, size(tmpImg,2)+pad);
H = fft2(double(h), size(tmpImg,1)+pad, size(tmpImg,2)+pad);
%Multiply the transform by the filter:
G=H.*F;
%Obtain the real part of the inverse FFT of G:
g=abs(ifft2(G));
%Crop the top, left rectangle to the original size:
strImg.pro=g(sub+1:size(g,1)-sub,sub+1:size(g,2)-sub);
else
warning off;
strImg.pro = im2double(imfilter(im2uint8(tmpImg),h,'symmetric','conv'));
warning on;
end
So i basically have about 80 of these case statements and i refuse to believe that this is the best way to do this. But since i don't have a lot of experience i'd rather ask you guys before i do anything stupid. Because if it really is the best option than i will simply leave it alone. But it looks pretty messy and gets kind of confusing especially if more if statements are added. Thanks a lot for your input and ideas!

Akzeptierte Antwort

Walter Roberson
Walter Roberson am 19 Sep. 2018
ismember() against a cell array of character vectors, take the second output, to find which of the vectors it is. Use that value to index a cell array of function handles, pull out that one function handle, and invoke it with appropriate arguments.
This requires that you define an interface of which variables each case is permitted to set.
  10 Kommentare
Stephen23
Stephen23 am 19 Sep. 2018
@Julian: you don't need main in the handle definition.
Julian
Julian am 19 Sep. 2018
The thing is i have it exactly like that;
functions = {@subimage; @subregion; @useROI;}
So no use of main at all. But still all of the functions that are defined later on in the part with all of the nested functions are stored with @main/... if i add a new function but won't yet define it in the part with the other nested functions and simply run it up to functions{@subimage; @subregion; @useROI; @newFunc} than only this handle won't get the @main/ .

Melden Sie sich an, um zu kommentieren.

Weitere Antworten (1)

Steven Lord
Steven Lord am 19 Sep. 2018
Instead of using ismember or a struct filled with function handles, I would move the code inside each of your case statements into their own function (a local function in that same file, a private in a directory named private, a function in a package directory, or a regular old function if it is sufficiently general that this code and others could use it.) Then you code would look like:
switch taskToPerform
case 'Use ROI'
strImg = performROI(fs, strImg);
case 'Blurring'
strImg = performBlurring(fs, strImg);
case ...
Organizing each task into their own separate functions lets you insulate those tasks from new variables that you may introduce in the main function later on, control how much and which information they can access from the main function, and (if defined as a package function or a regular old function) lets you test those functions in isolation so that you can be more confident that they perform correctly when used as part of your larger application.
As part of that refactoring, you may find that certain of your task functions share code. In that case, consider extracting those common operations into their own helper functions.
  1 Kommentar
Adam
Adam am 19 Sep. 2018
This is what I would do also. Removing the switch statement itself seems like it would obfuscate the meaning and create code that is a lot less intuitive, whereas moving out all the code inside, leaving just one-line function calls for each case, with a well-named function, seems to me a much more readable way to tidy up the code.

Melden Sie sich an, um zu kommentieren.

Kategorien

Mehr zu MATLAB Data API for C++ finden Sie in Help Center und File Exchange

Produkte


Version

R2018a

Community Treasure Hunt

Find the treasures in MATLAB Central and discover how the community can help you!

Start Hunting!

Translated by