Wondering why resharper would tell me this

Resharper is telling me to change the way I was doing some conditional statements.

THe line of code is within an XMLTextReader.Read() method where I go:

String c;
c = xReader["MyAttribute"] == null ? string.Empty : xReader["MyAttribute"].ToString();

-



It wanted me to go to:

c = xReader["MyAttribute"] ?? String.Empty;

-



The questions I have, is what does .NET Check against with the ?? opperator? It seems to me that telling the compiler to check to see if it's null, is more efficient because you're giving it specific instructions. The way resharper asks me to do it leaves it up to the compiler to make the decisions.


Also, it's also inneficient to send an object to a string variable, as xReader[""] returns an object and not a string.
Would it not make more sense to return xReader["MyAttribute"].ToString() vs xReader["MyAttribute"] ?

I don't mind coding 10x the amount of code, as long as everything is efficient and executes quickly (I don't care how convoluted my code looks either :P)

6 comments
Comment actions Permalink

Hello rternier,

I didn't quite understand all the issues you are talking about. Given the
following code:

void foo(string path)
{
XmlTextReader reader = new XmlTextReader(path);
string c = reader["MyAttribute"] == null ? string.Empty : reader["MyAttribute"].ToString();
}

All I see ReSharper suggesting is to remove ToString(), which is indeed redundant
- indexer on XmlReader (and XmlTextReader which inherits it) is of type string.
After you remove ToString(), you get context action on "?" sign, which allows
but not forces you to convert ternary (?:) to coalescing (??) operator.
By using coalescing you most likely will end up with better code, which is
both easier to read and faster, because each expression is known to evaluate
only once. With ternary, it depends on optimization power of compiler.

I don't see any problem here, may be you can post screenshot to explicitly
explain what's wrong?

Sincerely,
Ilya Ryzhenkov

JetBrains, Inc
http://www.jetbrains.com
"Develop with pleasure!"


r> Resharper is telling me to change the way I was doing some
r> conditional statements.
r>
r> THe line of code is within an XMLTextReader.Read() method where I go:
r>
r> String c;
r> c = xReader["MyAttribute"] == null ? string.Empty :
r> xReader["MyAttribute"].ToString();
r> -


r>
r> It wanted me to go to:
r>
r> c = xReader["MyAttribute"] ?? String.Empty;
r>
r> -


r>
r> The questions I have, is what does .NET Check against with the ??
r> opperator? It seems to me that telling the compiler to check to see
r> if it's null, is more efficient because you're giving it specific
r> instructions. The way resharper asks me to do it leaves it up to the
r> compiler to make the decisions.
r>
r> Also, it's also inneficient to send an object to a string variable,
r> as xReader[""] returns an object and not a string.
r>
r> Would it not make more sense to return
r> xReader["MyAttribute"].ToString() vs xReader["MyAttribute"] ?
r>
r> I don't mind coding 10x the amount of code, as long as everything is
r> efficient and executes quickly (I don't care how convoluted my code
r> looks either :P)
r>


0
Comment actions Permalink

There wasn't a problem, I wanted to know why - and whether it was more efficient to use the one resharper suggested.

Thank you for clearing it up.

0
Comment actions Permalink

By using coalescing you most likely will end up with better code,
which is
both easier to read and faster, because each expression is known to
evaluate
only once. With ternary, it depends on optimization power of compiler.
I don't see any problem here, may be you can post screenshot to
explicitly explain what's wrong?


Thanks for the answer to that Ilya. While we're on the topic, what are the
advantages of the optimisation where you can make an instance method static?

Thanks

Sean


0
Comment actions Permalink

Sean,

First of all, invocation of a static method is a little bit faster, for its
address does not need to be resolved at runtime. But eventually this gain is
negligible. Second, making method static improves code readability for it
becomes much easier to understand, that the method is independent from the
instance and may be move to a separate utility class.

--
Sergey V. Coox
JetBrains, Inc
http://www.jetbrains.com
"Develop with pleasure!"


0
Comment actions Permalink

Third, static members are a code smell. (Not one of the strongest smells, but a smell none-the-less.) Specifically they are an indicator of a lack of OO, and should generally only appear within pure utility classes.

0
Comment actions Permalink

Thanks Sergey and Jeremy, I'm still not convinced about this....it's a contender
for the first R# default that I may turn off....but I've not been sure for
a while, so I guess I'll watch the results for a while more. Ah, decisions......


0

Please sign in to leave a comment.