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

[Atsushi Eno]

Hey,

Well, my comments are more specific to the case that Rodrigo pointed
out. I in general agree with your preference on explicit cast over
"as" in theory.

Atsushi Eno

Juraj Skripsky wrote:
> 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
>>
> 
>