3

I am currently working on a project that has very little documentation overall. The team is working to change that. I am doing my part, by adding xml comments to the methods I make and the ones I edit, so that the automatic documentation generation tools can use them.

For some methods, complexity is either obvious or irrelevant.

But for some other methods, the way the code is built right now, they can seem like they are pretty light, but actually be very slow (talking about n³ and above here).

Of course, I am aware that there is an underlying problem and that refactoring is probably needed. But we neither have the time nor the budget to refactor everything within a reasonable delay, so in the meanwhile, I would like to communicate this information as a warning.

example:

/// <summary>
/// Tests if a thing is in a valid state. WARNING: This runs in O(n³)
/// </summary>
/// <returns>The validity of the thing</returns>
public virtual bool IsSomethingValid()
{
    return DoCodeThatIsMoreComplexThanItLooks();
}

I usually find those cases when doing some performance improvements, but at this point, this happens pretty often, and I think adding such big warnings too often will lose its impact.

So my idea would be to simply add it to the standard XML comment format, and put the complexity regardless of what it is, as a bit of information that may be useful or interesting.

Is there a standard way to communicate the complexity of a method in XML comments? like a tag that Visual Studio will understand (but doesn't add by default).

  • 1
    I think you are overestimating the importance of this. When your code is fast enough, it is probably not worth the hassle to document the runtime complexity always (at least in 95% of all cases), and if it is not, it is probably worth to be optimized. That leaves you with a few methods where you know it could be worth to optimize them in near time, and for those, you don't need a special tag, a standard comment like the one you showed us is probably fine. – Doc Brown Jan 23 at 21:16
  • In addition to Doc's comment, if there are so many methods to refactor and you can't do them all at once (understandably). I would go even further and mark the methods obsolete. [Obsolete("This method runs in O(n³) and should be refactored.")]. This will generate a real warning in Visual Studio which will be much more obvious to the users of the API. For methods that could benefit from small improvements in a later stadium, you could add // TODO comments, which will be automatially added to the Task list in Visual Studio. – Rik D Jan 23 at 21:25
  • 1
    Reading that comment, I immediately have the question "what's n here?". If you do go down that route, you should define it. That might end up as "This runs in O(k * n * m), where k ~ Foo.Count, n ~ Bar.Count, m ~ Baz.Count" – Caleth Jan 24 at 9:56
  • @RikD: I think that would be a misuse of the Obsolete attribute. Obsolete indicates that there is a better method which the user should use instead, not that the method needs refactoring. You will get a compiler warning at the call site which is not where the problem is. – JacquesB Jan 24 at 19:40
  • @JacquesB we have several tools at our disposal. How and when to use those tools depends on many things. In a public API you are most definitely right, it would be terrible to mark a method obsolete without a proper replacement. In an internal project, well, maybe this is the right tool for the job to alert your fellow developers if they need this method they should improve its performance first. It’s also probably a good idea to discuss this in your team first. – Rik D Jan 24 at 22:16
3

If the complexity is relevant for users of the method, then the best approach is to state it in the summary field as in your example. Of course it should clearly state what n represent, since it is not clear from the method signature.

C# has two kinds of comments, XML comments and regular comments. XML comments are used to generate documentation and are shown in tooltip help, code completion etc. So XML comments is documentation for the users of the the method. Regular comments inside code is for documentation of the implementation which is not relevant outside the method.

The question is when is complexity information relevant for a user? Only in the cases where the user would have the choice between using different algorithms with different trade-offs. But then you also need to state the other aspects of the trade-off (e.g large constant overhead), otherwise the user would just always select the one with the lowest complexity and there would be no purpose to having other options.

If the user does not really have a choice, then the complexity is pretty irrelevant. If you want to document it it should be in a regular comment inside the method.

For example what is the purpose of the warning in a validation method? Can the user just decide to skip the validation? Probably not.

  • In that case specifically, I wsa optimizing code, and I found a method that called the validation method once inside every iteration of a pretty big loop, while the loop had no effect on the result of the validation. I switched it to being called once at the beginning instead and saved a lot of time. I assume that the person didn't pay attention to where they were calling it, because it "looked like" it ran in O(1) anyway, from the signature only – Kaito Kid Jan 24 at 12:54
  • @KaitoKid: In that case the complexity of the validation method was irrelevant. The issue was it was called inside a loop instead of once, correct? – JacquesB Jan 24 at 15:44
  • That was the performance issue, yes. I am just trying to lower the odds of this kind of issue happening again, by looking into how it happened. I figured that this kind of documentation could help avoiding it. If not entirely, then at least make it happen less often. – Kaito Kid Jan 24 at 15:51
  • If the problem was unrelated to the complexity of the method then I don't think it is useful to add that information. Focus on what is relevant. If the problem was that the user misunderstood how the method should be used, then this is what the documentation should help avoid. – JacquesB Jan 24 at 16:01
  • Even if the user does not have a choice of which method to run, there may be a case where it could be run multiple times. If they know the method runs in O(n^3) rather than the expected O(1), they may wish to cache the results rather than run it again. Perhaps not likely, but a possibility... – mmathis Jan 24 at 20:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.