Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AV1562: Update for out var (exclude Deconstruct) and ref returns/locals #105

Closed
bkoelman opened this issue Oct 17, 2017 · 1 comment
Closed

Comments

@bkoelman
Copy link
Contributor

bkoelman commented Oct 17, 2017

Existing rule:

Don't use ref or out parameters (AV1562)
They make code less understandable and might cause people to introduce bugs. Instead, return compound objects.

New language features to consider:

  • out var
    The next example would be disallowed by this rule in its current form:
bool success = int.TryParse(text, out int number);
  • ref returns and ref locals
    This provides the ability to return by reference and to store references in local variables. Strictly speaking, this feature is not affected by this rule, but it is somewhat related. Example:
class Example
{
    private ref int GetMaxValue(ref int value1, ref int value2)
    {
        if (value1 > value2)
        {
            return ref value1;
        }

        return ref value2;
    }

    public void Test()
    {
        int number1 = 1;
        int number2 = 2;

        ref int max = ref GetMaxValue(ref number1, ref number2);
        max = 5;
        Console.WriteLine(number2); // prints 5
    }
}
  • Tuples and deconstruction
    The next example uses the compiler-recognized Deconstruct method (which must have out parameters), to enable deconstruction of the Point class into tuple elements.
public class Point
{
    public int X { get; }
    public int Y { get; }

    public Point(int x, int y)
    {
        X = x;
        Y = y;
    }

    public void Deconstruct(out int x, out int y)
    {
        x = X;
        y = Y;
    }
}

public class Example
{
    public void Test(Point point)
    {
        var (x, y) = point;
        Console.WriteLine($"Coordinates: ({x}, {y})");
    }
}

Proposal:

  • Add an exception to this rule for the TryParse pattern (and include example above).
  • Extend "Instead, return compound objects." to: "Instead, return compound objects or tuples."
  • ref returns/locals were introduced to bridge the performance gap with unmanaged unsafe code. It seems worth the effort only in unusual cases, so I don't think we should promote its usage. So let's not mention it.
  • Although CSharpGuidelinesAnalyzer should be made aware of the Deconstruct method and not report rule violations for it, I think we should not confuse readers by mentioning this feature here.
@dennisdoomen
Copy link
Owner

Agree with this proposal, in particular with the third bullet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants