Geek Answers Handbook

Are TryAction () methods "evil"?

I am trying to improve FxCop compliance in my code (for the first time in history), but I am a little fixated on the ethical issue. I have a GetText() method that returns a string from a remote server, but may throw an exception under certain circumstances. That's why I also have a TryGetText(ByRef text As String) method that returns a boolean indicating whether the call was successful. The return value is assigned to the text variable if true.

I thought this design was perfectly acceptable given that even Microsoft does this (e.g. Integer.TryParse ). FxCop tricks me into dictating, "You will not follow the link!"

To get around this warning (and there are quite a few), I replaced the StringBuilder parameter. But, despite the fact that now I am compatible, I do not think that he really improved my code.

Before:

  Public Function TryGetText(ByRef text As String) As Boolean Dim command As New GetTextCommand(Me) Dim result As CommandResult = ProcessCommand(command, True) If result.CommandStatus <> Constants.Status.Failed Then text = result.Text Return True Else Return False End If End Function

After:

  Public Function TryGetText(builder As Text.StringBuilder) As Boolean Dim command As New GetTextCommand(Me) Dim result As CommandResult = ProcessCommand(command, True) If result.CommandStatus <> Constants.Status.Failed Then builder.Clear() builder.Length = result.Text.Length builder.Append(result.Text) Return True Else Return False End If End Function

Is this an acceptable use of ByRef, or should I use an alternative to stringbuilder? It’s not very convenient for me to suppress this warning for every method where I use this construct. I don't feel the stringbuilder option improves code usability.

+4
source share
4 answers

Well, I think everyone hates too much byrefs. Two are already too many. Although there are many methods in your case, and they all follow the same pattern, this does not seem like a real problem. You have the option not to stick with the Microsoft Try * pattern.

Instead of boolean, why not return the string if successful, or Nothing / Empty if it failed? You can then test your TryGetText output with String.IsNullOrEmpty (resultText).

This is more code, really, but it resolves the warning (if that is really what you need).

0
source

This is probably a good use of ByRef , so I would be inclined to add an exception for this case. If you use the code analysis function in Visual Studio, you can simply add the following attribute to this method:

 <System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1045:DoNotPassTypesByReference", MessageId = "1#")> _ Public Function TryGetText(ByRef text As String) As Boolean
+2
source

TryAction methods are very useful in creating things like thread safe classes. It combines two stages, test and action, in one atomic operation. Microsoft makes heavy use of this in the Concurrent collection classes. For example http://msdn.microsoft.com/en-us/library/dd287191.aspx#Y0

So, I think that in the general case "You will not follow the link." should not be taken as the gospel in all cases. There is legal use. In your particular case, ByRef seems much less awkward than the version of StringBuilder. But if a simple string GetText() that returns null / Nothing when you are currently returning false is equivalent, this looks best.

+2
source

Passing by reference complicates the situation when you try to use your method in a functional style. You can use a type with a null value or a tuple or a custom parameter type.

Since I am not familiar with VB, here is an example of C #:

 struct Option<T> { public bool ContainsElement { get; private set; } private T element; public T Element { get { if (!ContainsElement) throw new NoElementException (); return element; } set { element = value; ContainsElement = true; } } public T GetElementOrDefault (T defaultValue) { return ContainsElement ? element : defaultValue; } } Option<string> GetText () { ... }
0
source

More articles:


All Articles