Best practice for passing structure to a function and adding new fields when returning it

This is more a style question than anything else. I'm passing a structure to a function, adding some fields to the structure, and returning the structure. Which of the following is considered best practice?

Option 1: defining the fields you want to add as function outputs, passing the structure to the function, then adding each field from the function outputs.

[structa.multiplied, structa.divided, ...
    structa.added, structa.subtracted, someExtraVar] = testFunction(structa, n);

function [multiplied, divided, added, subtracted, someExtraVar] = testFunction(structa,num)
    multiplied=structa.mat1.*structa.mat2;
    divided=structa.mat3./structa.mat4;
    added=structa.mat5+structa.mat6;
    subtracted=structa.mat6-structa.mat6;  
    someExtraVar=length(structa.mat6)*num;
end

or option 2: modifying the structure within the function

[structa, someExtraVar] = testFunction(structa);

function [structa, someExtraVar] = testFunction(structa, num)
    structa.multiplied=structa.mat1.*structa.mat2;
    structa.divided=structa.mat3./structa.mat4;
    structa.added=structa.mat5+structa.mat6;
    structa.subtracted=structa.mat6-structa.mat6;   
    someExtraVar=length(structa.mat6)*num;
end

Option 1 seems better to me, since the second option is modifying globals locally within function. The first option also tells you exactly what outputs the function returns without having to actually read the function. But I do have several functions which add multiple fields to a structure and it gets cumbersome to add four different fields from function outputs to a structure. It also does affect readability when a single line of code goes on forever.

Is there an accepted best practice for this/does anyone have any opinions or suggestions?

(This is dummy code, my actual structures and functions are more complex and I didn't want to distract from the question!)

Edit: a very simple snippet from my codebase because I think the above example isn't super clear about my actual usecase (I don't love the if switch, I might rewrtie that part later)

Option 1:

function [dat.filteredSignal,dat.filteredAtEvents,dat.muFSNew,...
    dat.sigmaFSNew,dat.muFSAtEvents,dat.sigmaFSAtEvents, ...
    dat.muFS, dat.sigmaFS] = normalizeFilteredSignals(dat,params)

    assert(or(params.normalizeFS==0,params.normalizeFS==1), 'normalize FS parameter must exist and be 1 or 0');

    filteredSignal=convertVarLengthCells2Mat(dat.filteredSignalEnsemble);
    filteredAtEvents=convertVarLengthCells2Mat(dat.filteredSignalAtEventEnsemble);

    [muFS,sigmaFS] = meanAndSigma(filteredSignal);
    
    if params.normalizeFS==1
        normFunc = @(x) (x-muFS)/sigmaFS ;
        filteredSignal=normFunc(filteredSignalAll);
        filteredAtEvents=normFunc(filteredAtEvents);

        [muFSNew,sigmaFSNew] = meanAndSigma(filteredSignal);
    else
        muFSNew=muFS;
        sigmaFSNew=sigmaFS;
    end
    
    [muFSAtEvents,sigmaFSAtEvents] = meanAndSigma(filteredAtEvents);

end

function [meanDat,sigmaDat] =   meanAndSigma(inputVec)
    meanDat=mean(inputVec,'omitnan');
    sigmaDat=std(inputVec,0,'all','omitnan');
end

Option 2:

function [dat] = normalizeFilteredSignals(dat,params)

    assert(or(params.normalizeFS==0,params.normalizeFS==1), 'normalize FS parameter must exist and be 1 or 0');

    filteredSignal=convertVarLengthCells2Mat(dat.filteredSignalEnsemble);
    filteredAtEvents=convertVarLengthCells2Mat(dat.filteredSignalAtEventEnsemble);

    [dat.muFS,dat.sigmaFS] = meanAndSigma(filteredSignal);
    
    if params.normalizeFS==1
        normFunc = @(x) (x-muFS)/sigmaFS ;
        dat.filteredSignal=normFunc(filteredSignalAll);
        dat.filteredAtEvents=normFunc(filteredAtEvents);

        [dat.muFSNew,dat.sigmaFSNew] = meanAndSigma(filteredSignal);
    else
        dat.muFSNew=muFS;
        dat.sigmaFSNew=sigmaFS;
        dat.filteredSignal=filteredSignal;
        dat.filteredAtEvents=filteredAtEvents;
    end
    
    [dat.muFSAtEvents,dat.sigmaFSAtEvents] = meanAndSigma(filteredAtEvents);

end

function [meanDat,sigmaDat] =   meanAndSigma(inputVec)
    meanDat=mean(inputVec,'omitnan');
    sigmaDat=std(inputVec,0,'all','omitnan');
end

[Torsten]

I'd prefer this (i.e. making changes to the structure only in the calling program), but it's a matter of taste.

structa.mat1 = 23;
structa.mat2 = 3;
structa.mat3 = 12;
structa.mat4 = -3;
structa.mat5 = -pi;
structa.mat6 = exp(1);
num = 10;

[multiplied, divided, added, subtracted, someExtraVar] = testFunction(structa,num);
structa.multiplied = multiplied;
structa.divided = divided;
structa.added = added;
structa.subtracted = subtracted;

structa

structa = struct with fields:
          mat1: 23
          mat2: 3
          mat3: 12
          mat4: -3
          mat5: -3.1416
          mat6: 2.7183
    multiplied: 69
       divided: -4
         added: -0.4233
    subtracted: 0

function [multiplied, divided, added, subtracted, someExtraVar] = testFunction(structa,num)
    multiplied=structa.mat1.*structa.mat2;
    divided=structa.mat3./structa.mat4;
    added=structa.mat5+structa.mat6;
    subtracted=structa.mat6-structa.mat6;  
    someExtraVar=length(structa.mat6)*num;
end

[Sam Chak]

Whether Style 1 (expansive) or Style 2 (compact) is preferable depends on user preferences and the requirements of the project. Is testFunction() a standalone function, or are its outputs intended to be updated or reused in a parent function or a main script containing loops?

Style 1 generally requires users to remember the order of the output arguments. For one-time execution involving only a few outputs, Style 1 may be more convenient for most users. Even so, I often confuse the azimuth and elevation angles when using [out1, out2] = view() to readjust the camera’s line of sight with view(out1 + value1, out2 + value2).

When there are many computed outputs, Style 2 is usually more practical. For example, simOut = sim(model) contains all the data logged during the simulation, as well as metadata describing the simulation. However, you can write a wrapper function based on Style 1 to determine how the data should be handled before making changes to the structure.

You might also want to combine the advantages of both styles by using nargout, which returns the number of output arguments requested by the caller of the currently executing function.

sys = tf([1], [4 3 2 1])

sys =
 
             1
  -----------------------
  4 s^3 + 3 s^2 + 2 s + 1
 
Continuous-time transfer function.

% Style 1
[out1, out2, out3, out4] = margin(sys)    % [Gm, Pm, Wcg, Wcp] = margin(sys)

Warning: The closed-loop system is unstable.

out1 = 0.5000

out2 = -25.3875

out3 = 0.7071

out4 = 0.7966

% Style 2
structA = stepinfo(sys)

structA = struct with fields:
         RiseTime: 2.4001
    TransientTime: 46.8491
     SettlingTime: 46.8491
      SettlingMin: 0.6750
      SettlingMax: 1.4493
        Overshoot: 44.9337
       Undershoot: 0
             Peak: 1.4493
         PeakTime: 6.3852

[Stephen23]

"Is there an accepted best practice for this/does anyone have any opinions or suggestions?"

There is no single "best" approach to this. Like everything to do with code, "it depends" is the the correct answer.

"since the second option is modifying globals locally within function"

Do not let some abstract rule-of-thumb override your concrete needs and requirements. A rule of thumb is only a rule of thumb, if you can justify why your task is better suited for some other design then use that other design.

Remember that correctness is always your first priority: if this is easier to justify with option 2, then that overrides some abstract rule of thumb that might provide 3ns runtime improvement vs. failing to ensure correctness which costs you hours/days/weeks of your actual progress instead.

Personally I would favor simplicity in your situation. Often when you try something you find that several other parts of your design simplify and refactor into neater code too, which is usually a good indications that you have found a good abstraction of the task at hand. So, try option 2 and see what happens.

[Oshani]

@Sam Chak , testFunction is something I made up! I edited my original post to include a simple example of what I'm talking about from my codebase. These functions are all being used inside a parent function. The parent function has zero inputs in the actual function handle (the user is prompted to load their data using a file dialog), and returns only figures as output.

That's useful information about nargout, thank you, will look into it!

[Oshani]

@Stephen23 thank you for this! I'm pretty new to coding anything more complicated than a 10-line script so I do struggle to figure out which 'rules' I can break and which I can't.

Answer 1

[Steven Lord]

With option 1, the user of your code needs to know and respect the order in which you have your function return its outputs. Suppose that instead of what you wrote:

[structa.multiplied, structa.divided, ...
    structa.added, structa.subtracted, someExtraVar] = testFunction(structa, n);

they felt that addition and subtraction were more "fundamental" operations than multiplication and division and so wrote:

[structa.added, structa.subtracted, ...
    structa.multiplied, structa.divided, someExtraVar] = testFunction(structa, n);

How long would it take before they detected that their answers didn't make sense? Would they detect that their answers didn't make sense? "Silent wrong answer" bugs are the most severe category of bugs at MathWorks; crashes are as severe but in some ways they're not as bad because if MATLAB crashes you know for a fact that there's a problem.

Option 1 also has a problem with extensibility. Suppose you realized later that you needed testFunction to also return the result of raisedtopower = structa.mat9.^structa.mat10. With option 1, as which output argument do you return structa.raisedtopower? If you return it anywhere prior to the sixth output you break existing code that was written with the first five outputs having definite purposes. If you return it as the sixth output, now you have someExtraVar in the middle of outputs representing fields and it's likely new users of your code will swap the fifth and sixth output argument (to group all the "structa.<something>" outputs together).

Of course, "Option 1 seems better to me just because the second is modifying globals locally within functions, but I do have several functions which add multiple fields to a structure and it does get cumbersome." suggests that your functions might be violating the Single Responsibility principle, trying to do too much at once.

Answer 2

[Matt J]

Which of the following is considered best practice?

I would say the most common (and therefore perhaps the best) practice is Option 2. It's essentially what you're always doing in object-oriented programming. There is little difference between struct field modification in a function and object property modification inside a class method.

Option 1 seems better to me, since the second option is modifying globals locally within function. The first option also tells you exactly what outputs the function returns without having to actually read the function.

As a preliminary remark, I don't know what "modifying globals" means in this context. The fields of a struct do not behave like global variables in any way that is intuitive to me.

Generally speaking though, the very purpose of a function is to hide the details of what it is doing from the calling routine, and the purpose of a struct is to hide the variables that it carries. You want to do that to reduce code clutter, and Option 1 works against that. If you need visual reminders of what a function call is doing, that is normally accomplished using code comments and help documentation.

Further visuals cues can come from choosing variable and function names that are suggestive of what the variable contains and what a function call is doing. In particular, reusing the name structa for the output of,

[structa, someExtraVar] = testFunction(structa, num);

is awkward, since the output structa has new fields and therefore substantially different composition from the input. Instead, I might do,

[augmentedStruct, someExtraVar] = augmentMatStruct( structa, num);