Too many ‘ref’ parameters¶
ID: cs/too-many-ref-parametersKind: problemSecurity severity: Severity: recommendationPrecision: very-highTags: - maintainability - readability - testabilityQuery suites: - csharp-security-and-quality.qls
Click to see the query in the CodeQL repository
Whilst passing method arguments usingref is occasionally a sensible thing to do (the canonical example is when writing a ‘swap’ method), overusing reference parameters can make methods needlessly difficult to understand and call.
Methods end up relying on their parameters being correctly initialized elsewhere and can have problems such as aliasing (when two parameters refer to the same object). They can be difficult to call because the references must refer to l-values (rather than temporary objects), so additional objects must be created to hold the results of the call. Their results can also be difficult to forward to other methods.
Recommendation¶
Whilst it is not applicable in every situation, and some judgment must be applied, a common solution is to create a new class that wraps the values you are trying to set in the method and then modify the method to return a new instance of it.
Example¶
In this example, populating thename,address andtel fields is done via a method that takesref parameters. This is not very good practice and makes the method confusing.
usingSystem;classBad{privatestaticvoidPopulateDetails(refstringname,refstringaddress,refstringtel){name="Foo";address="23 Bar Street";tel="01234 567890";}privatestaticvoidPrintDetails(stringname,stringaddress,stringtel){Console.WriteLine("Name: "+name);Console.WriteLine("Address: "+address);Console.WriteLine("Tel.: "+tel);}staticvoidMain(string[]args){stringname=null,address=null,tel=null;PopulateDetails(refname,refaddress,reftel);PrintDetails(name,address,tel);}}
It is better to wrap the values in their ownDetails class and then return a new instance ofDetails. It is easier to pass to other functions correctly and is easier to understand.
usingSystem;classGood{classDetails{publicstringName{get;privateset;}publicstringAddress{get;privateset;}publicstringTel{get;privateset;}publicDetails(stringname,stringaddress,stringtel){Name=name;Address=address;Tel=tel;}}privatestaticDetailsPopulateDetails(){returnnewDetails("Foo","23 Bar Street","01234 567890");}privatestaticvoidPrintDetails(Detailsdetails){Console.WriteLine("Name: "+details.Name);Console.WriteLine("Address: "+details.Address);Console.WriteLine("Tel.: "+details.Tel);}staticvoidMain(string[]args){PrintDetails(PopulateDetails());}}