Discussion on a custom rule, OverrideToStringMethodRule

25 views
Skip to first unread message

Lex Li

unread,
Nov 18, 2010, 8:27:11 AM11/18/10
to gend...@googlegroups.com
Hi guys,

I have been using this custom rule for my open source projects for a few months, and started to maintain it in a fork after Mono moved to GitHub.

https://github.com/lextm/mono-tools/commit/261ec39bb6d1ac72da7bb1d349e4e6737389c2c6#comments

Like spouliot commented, this rule is still far from mature. Any feedback is welcome.

Thanks,

Lex Li
http://www.lextm.com
"My King of Pop is dead"

Antoine V

unread,
Nov 18, 2010, 3:20:12 PM11/18/10
to Gendarme
Hello,

Maybe we can add a check for internal/private class to not apply the
rule if there is never a call to ToString (or parameter of
string.Format, Console.WriteLine).
The same way, if a call to ToString is detected, we can increase the
severity.

Antoine

Bevan Arps

unread,
Nov 18, 2010, 10:55:18 PM11/18/10
to gend...@googlegroups.com
I like the idea of a rule that promotes classes that "play well" in the debugger.

However, many of my own classes use the attribute [DebuggerDisplay] to control the string that is displayed by the debugger.

See http://msdn.microsoft.com/en-us/library/system.diagnostics.debuggerdisplayattribute.aspx

I'd suggest this rule shouldn't fire if the class has a [DebuggerDisplay] attribute defined.

Just my 2c,
Bevan.


Sebastien Pouliot

unread,
Nov 22, 2010, 9:44:04 PM11/22/10
to Gendarme
Hello Lex,

Thanks for bringing this up the mailing-list.

I think the rule has potential since I had a similar idea a while
ago ;-) What stopped me earlier was a feeling that it would bring a
lot of defects, quite a few of them not very important. For example:

a) in Gendarme most rules do not have any state so the best "ToString"
is still, IMO, the type name. That could be fixed easily by skipping
types without any fields (since most interesting properties will have
a, manual or automatic, backing field).

b) Abstract types do not really need a ToString since it won't be used
(assuming all inheritors provide their own). Now this does not play
well with (a) because the abstract type could have fields where the
concrete have none (not a very big issue and a parent-check could be
done too);

c) Bevan's comment about [DebuggerDisplay] is another case (but again
not perfect considering non-VS.NET users ;-)

I don't have more ideas right now but I'd be happy if those suggestion
(more feedback welcome) could be added to the rule. I think that would
make it something I like to fix for Gendarme.

Sebastien

p.s. one last thing let's move the rule from .Design
to .Maintainability


On Nov 18, 8:27 am, Lex Li <lextu...@gmail.com> wrote:
> Hi guys,
>
> I have been using this custom rule for my open source projects for a few
> months, and started to maintain it in a fork after Mono moved to GitHub.
>
> https://github.com/lextm/mono-tools/commit/261ec39bb6d1ac72da7bb1d349...
>
> Like spouliot commented, this rule is still far from mature. Any feedback is
> welcome.
>
> Thanks,
>
> Lex Lihttp://www.lextm.com

Sebastien Pouliot

unread,
Nov 22, 2010, 9:46:51 PM11/22/10
to Gendarme
That's a bit complicated and would miss some "honest" cases ;-)
e.g. list.Add (obj); foreach (object o in list) Console.WriteLine (o);

ToString are also very useful when debugging (in or out a debugger)
where the extra debugging code won't be present at analysis time.

Sebastien
Reply all
Reply to author
Forward
0 new messages