C# coding guidelines: use "this." or not when referring to member fields/properties within the obje

[Bill Butler]

[cut]

A good developer has this innate desire to write "elegant" code. But we must never over-indulge
that desire. This is one reason I put in so many hours that I am not paid for. I have my pride.
But I don't want my company to have to pay for it!

Interesting.

Turn this around and imagine your company publicly saying "We don't give our programmers enough
time or money to write code that they are proud of."

It doesn't sound so good if you put it like that does it?

I do a lot of maintenance of old code. Generally, I am tasked with:
- Making a small modification
- Making small fixes
- Troubleshooting a problem

I often have to fight the urge to rewrite the whole thing.(I don't always win)
It's not that we don't want to put out quality work. But, how do I justify spending a week on a task
that should take a day. If I come across a real timebomb in the code, I do suggest the rewrite to
the higher-ups

Generally, I attempt to be as elegant as I can be within reason.
If larger modifications are scheduled, I try to include "Rewritng" into my estimate.

Elegance is nice, but it does have to be justified if it will overly inflate the cost

Just my 2 cents
Bill

 

[Kevin Spencer]

Turn this around and imagine your company publicly saying "We don't give

our programmers enough time or money to write code that they are proud
of."

Not correct. I am a perfectionist by nature. Perfection is something
unattainable in this world. I *do* write code that they are proud of. I'm
just never satisfied with "good enough." It is *me* that I'm trying to
please in my spare time!

Like I said, "I have my pride..."

--
HTH,

Kevin Spencer
Microsoft MVP
Professional Numbskull

Hard work is a medication for which
there is no placebo.

[cut]

A good developer has this innate desire to write "elegant" code. But we
must never over-indulge that desire. This is one reason I put in so many
hours that I am not paid for. I have my pride. But I don't want my
company to have to pay for it!

Interesting.

Turn this around and imagine your company publicly saying "We don't give
our programmers enough time or money to write code that they are proud
of."

It doesn't sound so good if you put it like that does it?

 

[Brian Gideon]

Jeffery Richter has an annotation in the Framework Design Guidelines
book in which he recommends the m_ prefix as well. He also metions
that s_ is good prefix for static variables. I haven't adopted it yet,
but I'm seriously considering it.

 

[Jon Skeet [C# MVP]]

I would never write a big function for performance reasons
Goodo.


If there was a genuine reason to put them in a certain order, such as the
event handler for a menu then I would. But quite often there is no real
order.

In C# there pretty much always is - the order they're defined in the
enum, or the numeric order. Alphabetic for strings isn't quite so
useful, although it would still make it easier to find a particular
value when looking over the code.

What makes you think that reordering affects performance though? Have
you measured it (in C#, not VB6)?

The problem is you can't always step over. If the code is

DoSomething(MyProp);

then you can't step over MyProp and into DoSomething (afaik).

I *think* there's a way you can avoid it, but I can't remember it.

Besides I always forget to step over until I'm in.

If you think you won't want to step into those properties in general, I
believe there's an attribute you can apply to stop the debugger from
doing it. I'm blowed if I can find the name of it though

I've been programming in vb6 for many years and it's much easier in vb6 to
write incredibly inefficient code. For example IIf(x=1, 5 , 10) is 50 times
slower (I tested this once) than using an if statement because the first
returns a variant and the second just deals in ints. Vb6 is full of such
traps and it's very easy to write page after page of inefficient code so
I've become much more aware of such issues. I didn't realise until I had
this conversation just how much more efficient C# is (in some ways at
least). I could easily think of 50 examples in vb6 but struggled to find one
for C#. And the one I came up with wasn't very good I good example in
vb6 was I found out at one stage that "If len(SomeString) = 0" was much
faster than "If SomeString = "" " so I started using len. The 2 methods were
practically identical so why not use the faster.

Well, using the Length property is minisculy faster in C#/.NET too -
but personally I would still use if (x=="") because I find that easier
to read. Others find the .Length property easier to read, so they use
that - it's fine.

One example of where people go wrong is when they want to optimise loop
iteration. There are three obvious ways of iterating through a list or
an array - say of strings:

#1:
int len = list.Count;
for (int i=0; i < len; i++)
{
string x = list;
}

#2:
for (int i=0; i < list.Count; i++)
{
string x = list;
}

#3:
foreach (string x in list)
{
}

Now, people will use #1 because they think it makes it faster to only
look at the Count property once. In fact, #2 is faster than #1 (I
believe) because the JIT is able to make use of that common pattern and
optimise away the bounds checks - at least in some situations. (It may
be for arrays but not lists - I honestly can't remember at the minute,
and it may well change between CLR versions.)

However, #3 is the simplest and most readable form, if you don't need
the index itself. It says everything which is required and *only* those
things which are required.

Is it the most efficient? Nope. How often is that going to matter
though? The difference in performance is absolutely miniscule, so much
so that you'd only notice it at all if you had a virtually empty loop
and were iterating through millions and millions of times. At that
point, *if* you discover that's a bottleneck, that's the right time to
change to the most efficient form (assuming you can then prove that the
change does actually help). Until then, use the simplest form.

Don't worry, I don't go out of my way and make really bad designs to make
minor optimisations.

But do you go out of your way to make tiny changes to the code which
take it away from the path of clearest readability, just for the sake
of performance which is probably insignificant? How would you go about
iterating through a list in your normal code?

Generally the beginners code is slower and more buggy (and took them up to
10 times longer to write it). If you find performance issues with a more
experienced coder's work it usually harder to optimise it.

Well, it's likely to be harder to optimise the architecture, but you
may well be able to optimise the code itself, simply because plenty of
people take the same attitude I do.

That makes sense, the more loops a piece of code is inside the more critical
things become.

Sort of. I wouldn't be worried between a readable implementation and a
less readable one that was a constant 10% faster. When the *complexity*
is different, that's a real problem - and can get very ugly very
quickly, even if a loop isn't executed very many times.

For instance, suppose in one situation you have a loop which is
executed a million times, with a linear complexity. Changing the
implementation so that each iteration takes 10% less time will only
save 10% of the total time.

Now suppose you have a loop which is executed just 100 times, but it
becomes worse for each iteration so that the overall complexity is
O(n^3). Finding a way of making it O(n) will probably save *much* more
than 10% on the total time, even though the loop is only executed 100
times instead of a million.

 

[Michael C]

What makes you think that reordering affects performance though? Have
you measured it (in C#, not VB6)?

It has to do a comparison to each value presumably starting at the top.
Putting a commonly used item towards the bottom should result in more
comparisons.

I *think* there's a way you can avoid it, but I can't remember it.

You could use the DebuggerStepThroughAttribute.

If you think you won't want to step into those properties in general, I
believe there's an attribute you can apply to stop the debugger from
doing it. I'm blowed if I can find the name of it though

It used to be applied to the designer generated code in InitializeComponent
I believe but doesn't look like it is anymore.

Well, using the Length property is minisculy faster in C#/.NET too -
but personally I would still use if (x=="") because I find that easier
to read. Others find the .Length property easier to read, so they use
that - it's fine.

I don't think there's much difference in readability myself so use the
faster.

Now, people will use #1 because they think it makes it faster to only
look at the Count property once. In fact, #2 is faster than #1 (I
believe) because the JIT is able to make use of that common pattern and
optimise away the bounds checks - at least in some situations. (It may
be for arrays but not lists - I honestly can't remember at the minute,
and it may well change between CLR versions.)

Interesting. I would have used #1 when I needed the index.

But do you go out of your way to make tiny changes to the code which
take it away from the path of clearest readability, just for the sake
of performance which is probably insignificant? How would you go about
iterating through a list in your normal code?

Generally I would use #3 but wouldn't think it was the end of the world if
#1 or #2 was used when #3 could have been. It's possible you are
"micro-optimising" readability if you are using things like (X=="") instead
of len because of readability. I wouldn't consider readability down to that
level.

Michael

 

[Michael C]

#1:
int len = list.Count;
for (int i=0; i < len; i++)
{
string x = list;
}

#2:
for (int i=0; i < list.Count; i++)
{
string x = list;
}

#3:
foreach (string x in list)
{
}

Interesting, I done some testing and #3 was fastest (578ms), then #2 (469ms)
then #1 (406ms). The code I used is below.

Michael

private int[] _ints= new int[100000000];

private void button1_Click(object sender, System.EventArgs e)
{
int t = Environment.TickCount;
int count = _ints.Length;
for(int i = 0; i < count; i++)
{
_ints++;
}
MessageBox.Show((Environment.TickCount - t).ToString());

t = Environment.TickCount;
for(int i = 0; i < _ints.Length; i++)
{
_ints++;
}
MessageBox.Show((Environment.TickCount - t).ToString());

t = Environment.TickCount;
foreach(int i in _ints)
{
_ints[0] = i;
}
MessageBox.Show((Environment.TickCount - t).ToString());
}

 

[Michael C]

Interesting, I done some testing and #3 was fastest (578ms), then #2
(469ms) then #1 (406ms). The code I used is below.

Oops, times were back to front, should be:
#1 578ms
#2 469ms
#3 406ms

Michael

 

[Nick Hounsome]

Jeffery Richter has an annotation in the Framework Design Guidelines
book in which he recommends the m_ prefix as well. He also metions
that s_ is good prefix for static variables. I haven't adopted it yet,
but I'm seriously considering it.

It's a good idea to use both "this" and m_.

"this" gives you intellisense which elliminates typos and m_ makes it easier
to home in on your fields in the intellisense list.

Does anyone know if it is possible to make the form designer use a different
naming scheme for controls?

 

[Michael C]

It's a good idea to use both "this" and m_.

"this" gives you intellisense which elliminates typos and m_ makes it
easier to home in on your fields in the intellisense list.

I don't see the point in using m and _. One or the other differentiates
module level variables just fine.

Michael

 

[Nick Hounsome]

I don't see the point in using m and _. One or the other differentiates
module level variables just fine.

Michael

If you don't use a prefix then your fields will be all mixed up with your
methods and properties and also the methods and properties of your bases and
interfaces in the intellisense list.

I often forget what I've called my fields, particularly when designing forms
where there is always a LOT of base stuff in the intellisense list.

I just type this.m_ and I have a shortlist of the stuff I actually care
about to pick from.

 

[Michael C]

If you don't use a prefix then your fields will be all mixed up with your
methods and properties and also the methods and properties of your bases
and interfaces in the intellisense list.

I often forget what I've called my fields, particularly when designing
forms where there is always a LOT of base stuff in the intellisense list.

I just type this.m_ and I have a shortlist of the stuff I actually care
about to pick from.

You misread my post.

Michael

 

[Jon Skeet [C# MVP]]

It has to do a comparison to each value presumably starting at the top.
Putting a commonly used item towards the bottom should result in more
comparisons.

That's not how switch/case works - at least not when there are lots of
contiuguous entries. A table is used instead, to avoid doing that many
comparisons. When the entries are sparse, things get a bit trickier -
but I doubt that the performance is improved by moving things around. I
believe (after a short time of investigation, but nothing conclusive)
that the compiler will compile the code of the cases in the order you
specify, but do the comparisons in the same way whatever you do, just
jumping to different places. Of course, the JIT gets another stab at
optimisation too.

That's the thing with these micro-optimisations - they're *definitely*
not worth doing if you don't even know whether they'll help. The time
spent benchmarking this kind of thing (unless you happen to just be
interested and do it in spare time) is likely to be far more
"expensive" than any performance gains achieved.

You could use the DebuggerStepThroughAttribute.

That's the one!

It used to be applied to the designer generated code in InitializeComponent
I believe but doesn't look like it is anymore.


I don't think there's much difference in readability myself so use the
faster.

Well, there isn't much difference in readability - but there isn't much
difference is speed either. I think in most cases you're more likely to
waste a second of engineering time reading the code than you are to
waste a second of computing time running the slower code. I value
engieneering time higher than computing time.

Interesting. I would have used #1 when I needed the index.

Again, making the code a bit less readable for a minute difference in
speed, which may even be the wrong way!

Generally I would use #3 but wouldn't think it was the end of the world if
#1 or #2 was used when #3 could have been. It's possible you are
"micro-optimising" readability if you are using things like (X=="") instead
of len because of readability. I wouldn't consider readability down to that
level.

Whereas I view readability as absolutely king. Anything which
interrupts the flow of thoughts even slightly could cost time. In this
case, having an extra variable to think about could easily make someone
have to check something they wouldn't have to otherwise. (Heck, to
start with you need to check that the loop bounds look correct.)

The thing about this is that I don't have to deviate from natural
coding to do it - the most natural way of expressing the code when you
think of it in the first place is usually the most natural way of
reading it too. That's not true when you start bending the code to
achieve performance gains which will be negligible if they exist at
all.

 

[Christoph Nahr]

Interesting, I done some testing and #3 was fastest (578ms), then #2 (469ms)
then #1 (406ms). The code I used is below.

foreach is fastest in your test case because you're using an array.
For collections that implement ICollection but also have a fast
indexer (such as List<> or ArrayList) foreach would be slower than
using an indexed loop. On the other hand, for collections with a slow
indexer but a fast enumerator the foreach loop would be faster...

 

[Nick Hounsome]

You misread my post.

Ooops.

Personally I find that m_ is clearer than just m and I find _ ugly,
particularly next to "." .

 

[Willy Denoyette [MVP]]

| | > Interesting, I done some testing and #3 was fastest (578ms), then #2
| > (469ms) then #1 (406ms). The code I used is below.
|
| Oops, times were back to front, should be:
| #1 578ms
| #2 469ms
| #3 406ms
|
| Michael
|
|

Not surprising when you do this :
_ints[0] = i;

in #3, instead of this:
_ints++;
like in #1 and #2.
These are two different operations, which invalidates the results.
Another thing you need to do is to warm-up the caches, before you run
micro-benchmarks like this, now #1 takes the hit of warming-up the caches,
so you need to run #1 once before actually timing.

With both corrections in place I measured:
#1: 226 ms
#2: 225 ms
#3: 284 ms

See #1 and #2 are (obviously) equaly fast, while #3 is the slowest.

Compare this to:
#1: 349 ms
#2: 213 ms
#3: 150 ms
.... when ran without the corrections.
See how easy it is to draw wrong conclusions when running micro benchmarks?

Willy.

 

[Mehdi]

It's a good idea to use both "this" and m_.

"this" gives you intellisense which elliminates typos and m_ makes it easier
to home in on your fields in the intellisense list.

In case some are not aware, in VS 2003, instead of using "this." to get the
intellinsense pop-up you can simply press Ctrl-space. This is probably the
most essential shortcut in this version of VS.

 

[Michael C]

Ooops.

Personally I find that m_ is clearer than just m and I find _ ugly,
particularly next to "." .

_ is ugly but I don't use it next to . There's no reason to prefix a module
level var with this if it starts with an _ because the _ tells you it is
part of this. I think m_ is uglier personally while just a plain m is the
least ugly although I don't use it.

Michael

 

[riscy]

This is very interesting read, however in my view, warming up the cache
should be part ot the timing measurement because 'warming up' is part
of the loop coding. Unless you have to very good reason why to excludes
it.

But it is very interesting observation....

In the end of the day, PC is so fast, the micro-benchmark may not be
essential as it use to be with 8080!.

It was a pity the window boot process is still slow, regardless how
powerful the processor can be!.

Riscy

 

[Willy Denoyette [MVP]]

| This is very interesting read, however in my view, warming up the cache
| should be part ot the timing measurement because 'warming up' is part
| of the loop coding. Unless you have to very good reason why to excludes
| it.
|

If you don't warmup the caches (L1 instruction/data), you favor the test #2
over #1, leading to false results. Another observation is that the GC kicks
in during the first test run, this is because the Gen0 threshold is in
general reached after the process initialized the CLR and brought in the
initially required objects. So normally you should force a GC as well after
you ran the first test to warmup the caches.
So the sequence I've used looks like:
run #x
GC.Collect
start timeing
run #1
stop timing
start timing
run #2
stop timing
....

| But it is very interesting observation....
|
| In the end of the day, PC is so fast, the micro-benchmark may not be
| essential as it use to be with 8080!.
|

Note that I don't care at all about the results of micro benchmarks like
these, they have no value other than prove that one construct may be
somewhat faster than another when run standalone, but the speed differences
fade away rapidly in a real world application.

| It was a pity the window boot process is still slow, regardless how
| powerful the processor can be!.
|

Take care, while processors are getting faster every day, other crucial
resources are not.
Especially the memory system is becomming a real issue.

Willy.

| Riscy
|
| Willy Denoyette [MVP] wrote:
| > | > | | > | > Interesting, I done some testing and #3 was fastest (578ms), then #2
| > | > (469ms) then #1 (406ms). The code I used is below.
| > |
| > | Oops, times were back to front, should be:
| > | #1 578ms
| > | #2 469ms
| > | #3 406ms
| > |
| > | Michael
| > |
| > |
| >
| > Not surprising when you do this :
| > _ints[0] = i;
| >
| > in #3, instead of this:
| > _ints++;
| > like in #1 and #2.
| > These are two different operations, which invalidates the results.
| > Another thing you need to do is to warm-up the caches, before you run
| > micro-benchmarks like this, now #1 takes the hit of warming-up the
caches,
| > so you need to run #1 once before actually timing.
| >
| > With both corrections in place I measured:
| > #1: 226 ms
| > #2: 225 ms
| > #3: 284 ms
| >
| > See #1 and #2 are (obviously) equaly fast, while #3 is the slowest.
| >
| > Compare this to:
| > #1: 349 ms
| > #2: 213 ms
| > #3: 150 ms
| > ... when ran without the corrections.
| > See how easy it is to draw wrong conclusions when running micro
benchmarks?
| >
| > Willy.
|

 

[Michael C]

In case some are not aware, in VS 2003, instead of using "this." to get
the
intellinsense pop-up you can simply press Ctrl-space. This is probably the
most essential shortcut in this version of VS.

Ctrl space dates back to visual basic 5 and possibly C before that.

Michael