Feature Request: Detecting poor initializations.

[Ken Lassesen]

I just blogged about a code review item that is likely common but not always easy to spot

Instead of:
_TotalOperations = new PerformanceCounter();
                _operationsPerSecond.CategoryName = _categoryName;
                _operationsPerSecond.CounterName = "# operations executed";
                _operationsPerSecond.MachineName = ".";
                _operationsPerSecond.ReadOnly = false;
                _operationsPerSecond.RawValue = 0;

The following should be suggested:

_TotalOperations = new PerformanceCounter(){CategoryName = categoryName,CounterName = "# operations executed",ReadOnly = false}

The rest of the assignments above are redundant because they are the default values.

[Tsviatko Yovtchev]

Hi Ken,

 That's a good point. We'll add that feature. Don't hesitate to let us know if you have other suggestions, too.

All the best,

Tsviatko Yovtchev

the Telerik team

Instantly find answers to your questions on the new Telerik Support Portal.
Watch a video on how to optimize your support resource searches and check out more tips on the blogs.

[Ken Lassesen]

A related but additional feature was added to my blog.

The basic rule is:

Custom Initializers should be used when one of the following is true only:

• Passes in data that that is not exposed as a property
• Passes in data that is a property that is a private set.

To restate is in a simpler way:

If

var x=new StudentRecord( a,b,c,d);

can be rewritten as:

var x=new StudentRecord( a){paramb=b, paramc=c,paramd=d};

but cannot be rewritten as

var x=new StudentRecord(){ parama= a, paramb=b,paramc=c,paramd=d};

Then

• StudentRecord(object a) is appropriate
• StudentRecord(object a, object b, object c, object d) is inappropriate, being redundant.

[Tsviatko Yovtchev]

Hi Ken,

This is a reasonable feature. However, the number of optimizations at this level makes it impossible for us to implement all of them while keeping our UI of reasonable size. 

Greetings,
Tsviatko
the Telerik team

Instantly find answers to your questions on the new Telerik Support Portal.
Watch a video on how to optimize your support resource searches and check out more tips on the blogs.

[Stuart Hemming]

Ken,

Mind if I ask a question? You said ...

> Custom Initializers should be used when one of the following is true only:

• > Passes in data that that is not exposed as a property

Why do think this is a reasonable rule?

Isn't the implication of this that after calling the object initializer with any values that meant the rules you specify, it may then be necessary to explicitly set any values exposed as properties separately?

If I've misunderstood, I apologies, but, as I've read it, I can't see the sense in this.

--
Stuart

[Ken Lassesen]

Consider the following functionally equivalent pieces of code:

CODE A

_TotalOperations = new PerformanceCounter(){CategoryName = categoryName,CounterName = "# operations executed",ReadOnly =false}

CODE B

_TotalOperations = new PerformanceCounter(categoryName, "# operations executed", false);

CODE C

_operationsPerSecond = new PerformanceCounter();
_operationsPerSecond.CategoryName = _categoryName;
_operationsPerSecond.CounterName = "# operations / sec";
_operationsPerSecond.MachineName = ".";
_operationsPerSecond.ReadOnly = false;
_operationsPerSecond.RawValue = 0;

I'm advocating CODE A is desired. The benefits are:

• There is only one initializer written (reduction of lines of code -- reduces bug risks, code review time, etc). Code B requires at least 2.
• It is far clearer then with B  on what is being assigned than Code A.  With B you see values only, with A you see the name-value pairs.
• Code A pattern actually gives you N! one line initialiations where N is the number of read/write parameters that the class has.

          _TotalOperations = new PerformanceCounter(){CategoryName = categoryName};
          _TotalOperations = new PerformanceCounter(){ReadOnly =false};

          etc.

Does my reasoning make better sense now?

[Stuart Hemming]

Ken,

> Does my reasoning make better sense now?
Please don;t misunderstand me, I'm with you on the use of Object Initializers. It was the statement that ...

> Passes in data that that is not exposed as a property

The inference being that if the data /can/ be passed in it /shouldn't/ be passed in the Object Initializer.

Or have I just failed to read correctly the statement you initially made?

--
Stuart