Refactor a huge Switch Statement (1400 lines of Code)
13 vues (au cours des 30 derniers jours)
Afficher commentaires plus anciens
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!
0 commentaires
Réponse acceptée
Walter Roberson
le 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 commentaires
Plus de réponses (1)
Steven Lord
le 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 commentaire
Adam
le 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.
Voir également
Catégories
En savoir plus sur MATLAB Data API for C++ dans Help Center et File Exchange
Community Treasure Hunt
Find the treasures in MATLAB Central and discover how the community can help you!
Start Hunting!