[Mono-dev] [PATCH] The big "as" cast cleanup

[Juraj Skripsky]

Hi Atsushi,

I agree that the preference of the explicit cast is a matter of personal
taste in almost all parts of my patch. If the Mono team or a maintainer
decides against those changes, I'll drop the patch or those parts of it
where the generated code shows no advantages to the current source. No
problem.

I also find e.g.

"res = values[i] as string;"

easier to read than

"res = (string)values[i];"

But after having been bitten a few times by the
NullRefExc-instead-of-InvCastExc problem, I started to prefer to
explicit version wherever possible. The control freak in me likes to
code as strictly checked as possible.

Also, when reading other peoples' code, I find the second version to
convey more information. The first version tells me than "values[i]"
should contain null or string, but it could contain any other type as
well. The second one tells me to expect only null or a string.

But hey, I'm the type of guy who would love to replace all private
Hashtable members in the class libs by their respective Dictionary<T>
counterparts (with #ifdef NET_2 of course) if I had it my way :-).

- Juraj




On Wed, 2008-01-09 at 00:52 +0900, Atsushi Eno wrote:
> It is very strange argument. If there is a premise that the explicit
> cast should be no problem here, then as operator should be no problem
> as well.
> 
> I don't like to change working code unnecessarily just for your
> preference. I'd wait for another patch that removes any extra changes.
> 
> Personally I like as operator which sometimes makes the sources
> more readable and kind enough for code completion depending on
> the editor/IDE.
> 
> Atsushi Eno
> 
> 
> Juraj Skripsky wrote:
> > Hi Rodrigo,
> > 
> > SelectSingleNode ("cp") returns an XmlElement if an element <cp> is
> > found or null otherwise. So the explicit cast should be no problem here.
> > XPaths like "@someAttribute" or "count(...)" would of course be a
> > different story.
> > 
> > - Juraj
> > 
> > 
> > On Tue, 2008-01-08 at 12:25 -0200, Rodrigo Kumpera wrote:
> >> Hi Juraj,
> >>
> >> @@ -326,7 +324,7 @@
> >>                          Console.Error.WriteLine ("WARNING: {0} is not
> >> supported for now.", el.FirstChild.LocalName);
> >>                          continue;
> >>                      } 
> >> -                    XmlElement cpElem = el.SelectSingleNode ("cp") as
> >> XmlElement;
> >> +                    XmlElement cpElem =
> >> (XmlElement)el.SelectSingleNode ("cp");
> >>                      string v = ""; 
> >>                      if (cpElem != null)
> >>                          v = new string ((char) (int.Parse (
> >>
> >>
> >> Here there is an explicit test if cpElem is null, this could mean that
> >> the code expect the cast to fail. Isn't that a possibility? 
> >>
> >>
> >> Rodrigo
> >>
> >>
> >> On Jan 8, 2008 11:49 AM, Juraj Skripsky <js at hotfeet.ch> wrote:
> >>         Hello,
> >>         
> >>         After getting annoyed by NullReferenceExceptions which turned
> >>         out to be
> >>         ClassCastExceptions in disguise, I've decided to clean up the
> >>         code in
> >>         mcs/class/corlib with regard to wrong/unnecessary uses of the
> >>         "as" cast 
> >>         operator.
> >>         
> >>         Reasons for the clean up:
> >>         -------------------------
> >>         - NullRefExceptions are thrown where ClassCastException should
> >>         be. With
> >>         explicit casts, the later is thrown at the right spot and
> >>         gives you more 
> >>         info.
> >>         - Personally, I consider the use of the "as" operator to be
> >>         confusing /
> >>         irritating in those cases where a explicit cast is sufficient.
> >>         
> >>         The following links also describe the problems with the "as"
> >>         operator: 
> >>         http://www.winterdom.com/weblog/2006/09/27/CastingOperatorsAndToString.aspx
> >>         http://blog.mattwynne.net/2007/09/04/casting-with-as-in-c/
> >>         
> >>         The performance of explicit casts and "as" casts are pretty
> >>         much the
> >>         same on Mono and MS.NET (see and run attached test program). 
> >>         
> >>         Attached is a patch (including ChangeLog entries).
> >>         All unit tests pass.
> >>         
> >>         Please review.
> >>         
> >>         - Juraj
> >>         
> >>         _______________________________________________
> >>         Mono-devel-list mailing list 
> >>         Mono-devel-list at lists.ximian.com
> >>         http://lists.ximian.com/mailman/listinfo/mono-devel-list
> >>         
> >>
> >> _______________________________________________
> >> Mono-devel-list mailing list
> >> Mono-devel-list at lists.ximian.com
> >> http://lists.ximian.com/mailman/listinfo/mono-devel-list
> > 
> > _______________________________________________
> > Mono-devel-list mailing list
> > Mono-devel-list at lists.ximian.com
> > http://lists.ximian.com/mailman/listinfo/mono-devel-list
> > 
> 
> _______________________________________________
> Mono-devel-list mailing list
> Mono-devel-list at lists.ximian.com
> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>