I suggest you ...

Improve code analysis

Improve code analysis
- Detect when first line of if block and else block are identical (same for last line)
- Detect methods used only one time - Method is a candidate for in-lining
- Detect constant defined in class used only one time
- Detect private property with backing variable having get { return backingVariable; } set {backingVariable=value;} - convert to autoproperty
- Detect public property used only within the same class - make it protected/private
- Detect public method used only within the same class - make it protected/private
- Detect class used only by one other class - class may be nested
- Detect unused references in a project
...
Including a few of these 10 to 20 every release of VS would help greatly.

850 votes
Vote
Sign in
Check!
(thinking…)
Reset
or sign in with
  • facebook
  • google
    Password icon
    I agree to the terms of service
    Signed in as (Sign out)
    You have left! (?) (thinking…)
    RR shared this idea  ·   ·  Flag idea as inappropriate…  ·  Admin →

    11 comments

    Sign in
    Check!
    (thinking…)
    Reset
    or sign in with
    • facebook
    • google
      Password icon
      I agree to the terms of service
      Signed in as (Sign out)
      Submitting...
      • RR commented  ·   ·  Flag as inappropriate

        Source code static analysis is to find possible errors, identify code improvements, measure complexity amongst others.

        The suggestions, including the second set in the comments, benefit the original developer and, more importantly, the second developer to work on the code base.

        This lowers the lifetime cost of a software solution be reducing the developer time both in the 20% initial development and the 80% or more spent after the first production release.

        Much of the 80% cost after the first production release is discovering the business implemented by reading/debugging the source code.

        That is why static analysis is distinct and not connected to the never read by humans output of an optimizing compiler.

      • André WerlangAndré Werlang commented  ·   ·  Flag as inappropriate

        @R, I agree that this is useful information but most of this is really job of an optimizing compiler. We should focus on adding value to our programs instead of doing almost useless refactorings.

      • PhilippePhilippe commented  ·   ·  Flag as inappropriate

        - Finding methods that are used once to “inline” them is a bad idea. In fact, to help code clarity, you often want to do the opposite. Split large methods in multiple functions even if the code is used only from that function. It will also help code reuse in the future as those smaller functions would be more easily reusable.

        - Moving constants from class level to method level is more often that not a bad idea.

        - Nesting classes should be applied with discretion. In pratice, it should only be done if the class is private/internal and used only by the outer class and even then, you have to be careful with that if name are used in settings, XML or by name (in strings).

        - Converting to autoproperty is essentially useless. Ne real benefit when the code is already written.

        Thus in the end, most of these are not that much benefical... And other like detecting that the first line of an if is identical to the first line of the else branch, any competent programmer should do it anyway.

      • JenJen commented  ·   ·  Flag as inappropriate

        VS will never have decent code refactoring.

        That would have an impact on ReSharper sales, which would make JetBrains add C# to their IDEs, and that would be the death of Visual Studio.

        You have my vote!

      • Anonymous commented  ·   ·  Flag as inappropriate

        - Detect when the last statement in a method is a return
        Refactoring: Remove the redundant return

        - Detect when the if path of an if then else statement ends with a return
        Refactoring: Remove the else keyword, curly braces and do not nest the else part

        - Detect Interface defined in a solution and implemented by only one class in that same solution
        Refactoring: Remove the interface if it is never to be exposed outside of the solution

      • AnonymousAnonymous commented  ·   ·  Flag as inappropriate

        Also, detect when object initializer can be used for collection based objects.
        IList<int> tmp = new IList<int>();
        tmp.Add(1);
        tmp.Add(2);
        ...
        tmp.Add(99);

        can be refactored into a single line and avoide all of the Add() method calls.
        IList<int> tmp = new IList<tmp>() { 1, 2, ....., 99 };

        Detect unnecessary 'else' such as
        if (...)
        {
        x = 5;
        return;
        }
        else
        {
        x = 7;
        }

        Refactors into

        if (...)
        {
        x = 5;
        return;
        }

        x = 7;

        This also applies for continue statements inside of loops.

      • Martijn HoekstraMartijn Hoekstra commented  ·   ·  Flag as inappropriate

        I like much of this, but they all have valid use cases, so if any of these are implemented, they should require manual activation runs, as they could have false positives, and the result should never be bad enough to be a warning (but it would be handy to have an information item on them). Some points that haven't been touched on yet:

        - Detect when first line of if block and else block are identical (same for last line) : This would be really nice

        - Detect methods used only one time - Method is a candidate for in-lining
        - Detect constant defined in class used only one time : I wouldn't even want a note for these, this is just good practice

        - Detect public property used only within the same class - make it protected/private
        - Detect public method used only within the same class - make it protected/private : As touched upon by others. However, if you are the primary consumer of your own library, it could be nice to know which properties/methods you're not touching within a larger project. They could well be candidates for making them less priviledged, even if in some/many/most (depending on the library) cases they aren't

        - Detect unused references in a project : This may be needed to satisfy dependencies for dynamically loaded classes.

      • RR commented  ·   ·  Flag as inappropriate

        The goal is to Identify possible code simplifying refactorings to be done by the developer.
        This is outside of the normal data flow optimizations done by the compiler.

        - Detect when first line of if block and else block are identical (same for last line)
        Refactoring: For first line, move common line before if statement. For last line, move statement after end of else block.

        - Detect methods used only one time - Method is a candidate for in-lining
        Refactoring: Move body of method called one time inside of caller's body if caller is not too complex or long (developer's decision). Reduce complexity of items visible at class scope.

        - Detect constant defined in class used only one time or used only in one method
        Refactoring: Move constant inside of the method using it. Reduce number of items visible at class scope.

        - Detect private property with backing variable having get { return backingVariable; } set {backingVariable=value;} - convert to autoproperty. VS 2012 already does this.

        - Detect public property, method or constant used only within the same class - make it protected/private
        Refactoring: Make it private/protected

        - Detect class used only by one other class - class may be nested
        Refactoring: Nest class and make it private. Reduce number of global signatures without increasing code complexity.

        - Detect unused references in a project
        Refactoring: Remove referenced DLLs, projects from project

        - Detect unused usings in C# file
        Refactoring: Remove unused usings in C# file. It helps in global impact analysis for large systems having multiple solutions each with multiple projects. Find all references stops at the currently open solution which leads to full text search of entire source tree of a multiple solution .system.

        - Detect methods that may be made static.

        - Detect property, class member variable used by only one method.
        Refactoring: Use a local variable in the method. Reduce complexity by reducing number of things visible at class level scope.

        - Detect line/value that is constant in a loop. This is the same as the top/bottom factoring of the if statement.
        Refactoring: Move statement before the start of the loop

        - Detect foreach loop over a collection having an operation inside the loop that modifies the collection. This can generate a 'Collection modified while being enumerated' exception.
        Refactoring: Do not modify collection inside foreach loop. Or, less appealing, use a for loop.

        - Detect if statements that may be inverted to reduce nesting.
        Refactoring: Invert if statements to reduce nesting

        public void MethodA(string a)
        {
        if (a != null)
        {
        if (a == "3")
        {

        ... more than a a few lines of code
        }
        }
        }

        Convert to

        public void MethodA(string a)
        {
        if (a == null)
        return;

        if (a != "3")
        return;

        ... more than a a few lines of code
        }

      • PauloPaulo commented  ·   ·  Flag as inappropriate

        I like the first idea.
        For the second idea (inlining) that must be done automatically by compiler or even by the jitter, but it goes against good practices (ie. splitting large methods into smaller ones).

        About the others... well, I think only the detect unused references is useful.

      • AnonymousAnonymous commented  ·   ·  Flag as inappropriate

        Suggestions are reasonable for large over-modularized code bases, 500k lines of code or more. Less identifiers visible at each line of code helps.

      • Anonymous commented  ·   ·  Flag as inappropriate

        - Detect methods used only one time - Method is a candidate for in-lining = bad idea*
        - Detect constant defined in class used only one time = bad idea*
        - Detect private property with backing variable having get { return backingVariable; } set {backingVariable=value;} - convert to autoproperty = only if the field is not used within the class
        - Detect public property used only within the same class - make it protected/private = impossible for dll's. you dont know what is used outside of the project/solution
        - Detect public method used only within the same class - make it protected/private = impossible for dll's. you dont know what is used outside of the project/solution
        - Detect class used only by one other class - class may be nested = bad idea*
        - Detect unused references in a project. = could be done, no big advantage.

        * = I suggest you read clean code or a similar book about code readability. Method, class and constant names semantics to your program and should not be removed because they are used only once. Inlining should be handled by the compiler.

      Feedback and Knowledge Base