Feature Request: Detecting poor initializations.

7 posts, 0 answers
  1. Ken Lassesen
    Ken Lassesen avatar
    37 posts
    Member since:
    Nov 2008

    Posted 11 Jan 2010 Link to this post

    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.
  2. Tsviatko Yovtchev
    Admin
    Tsviatko Yovtchev avatar
    408 posts

    Posted 12 Jan 2010 Link to this post

    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.
  3. DevCraft banner
  4. Ken Lassesen
    Ken Lassesen avatar
    37 posts
    Member since:
    Nov 2008

    Posted 12 Jan 2010 Link to this post

    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.

  5. Tsviatko Yovtchev
    Admin
    Tsviatko Yovtchev avatar
    408 posts

    Posted 13 Jan 2010 Link to this post

    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.
  6. Stuart Hemming
    Stuart Hemming avatar
    1622 posts
    Member since:
    Jul 2004

    Posted 14 Jan 2010 Link to this post

    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
  7. Ken Lassesen
    Ken Lassesen avatar
    37 posts
    Member since:
    Nov 2008

    Posted 14 Jan 2010 Link to this post

    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?
  8. Stuart Hemming
    Stuart Hemming avatar
    1622 posts
    Member since:
    Jul 2004

    Posted 14 Jan 2010 Link to this post

    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
Back to Top
DevCraft banner