Tagged: code analysis Toggle Comment Threads | Keyboard Shortcuts

  • danielsaidi 4:12 pm on December 8, 2011 Permalink | Reply
    Tags: circular namespace dependencies, code analysis, implementation, ,   

    NDepend getting my juices flowing 

    I have been using NDepend to analyse the latest version of .NExtra. The code is spotless (this is where you should detect the irony), but the analysis is highlighting some really interesting design flaws that I will probably fix in the next major version.

    First of all, almost all cases where method or type names are too long are where I have created facades or abstractions for native .NET entities, like Membership. Here, I simply add exceptions where NDepend will not raise any warnings, since the whole point with the facades are that they should mirror the native classes, to simplify switching them out with your own implementations, mocks etc.

    Second, NDepend warns for naming conventions that I do not agree with, such as that method names should start with m_. Here, I simply remove the rules, since I do not find them valid. However, when I look at it after removing the rules, I could have kept them and renamed them and make them check that my own conventions are followed. I will probably do so later on.

    Third, and the thing I learned the most from today, was that I turned out to have circular namespace dependencies, despite that I have put a lot of effort into avoiding circular namespace and assembly dependencies. The cause of the circular dependencies turned out to be between X and X/Abstractions namespaces.

    Circular dependency graph

    The base and abstraction namespaces depend on eachother

    For instance, have a look at NExtra.Geo, which contains geo location utility classes. The namespace contains entities like Position and enums like DistanceUnits, as well as implementations of the interfaces in the Abstractions sub namespace.

    Now, what happens is that the interfaces define methods that use and return types and entities in the base namespace. At the same time, the implementations in the base namespace implement the interfaces in the Abstractions namespace. And there you go, circular namespace dependencies.

    Now, in this particular case, I do not think that it is too much of a deal, but it highlights something that has annoyed me a bit while working with the .NExtra library.

    In the library’s unit tests, the test classes only knows about the interfaces, but the setup method either selects a concrete implementation or a mock for the various interfaces. For an example, look here. This force me to refer both the base namespace as well as the abstractions namespace. Once again, this is not that bad, but it raises the question…

    In the next minor version of .NExtra, I will probably get rid of the abstraction namespaces, since they add nothing but complexity. Sure, they tuck away all interfaces, but why should they?

     
    • Daniel Lee 6:40 pm on December 8, 2011 Permalink | Reply

      Were you inspired by Greg Young’s session at Öredev? I have to get into NDepend as well. Did you buy a license or you testing out the trial version?

    • danielsaidi 9:30 am on December 9, 2011 Permalink | Reply

      I have been using NDepend for a while, but yeah, Greg’s session opened up my eyes for other ways of looking at the metrics. And even if I remove some metrics (like methods should start with m_, static members with s_ etc.) it is really striking how some of the metrics really highlight design flaws that are easily corrected.

  • danielsaidi 9:16 am on October 6, 2011 Permalink | Reply
    Tags: code analysis, cql, , static   

    Tweaking the NDepend CQL rules to leverage awesome power 

    I have previously written about automating and scheduling NDepend for a set of .NET solutions.

    After getting into the habit of using NDepend to find code issues instead of going through the code by hand (which I still will do, but a little help does not hurt), the power of CQL grows on me.

    For instance, one big problem that I have wrestled with is that our legacy code contains static fields for non-static-should-be properties. In web context. Enough said.

    Prior to using CQL, I used to search for “static” for the entire solution, go through the search result (which, naturally, also included fully valid static methods and properties) and…well, it really did not work.

    Yesterday, when digging into the standard CQL rules to get a better understanding of the NDepend analysis, I noticed the following standard CQL:

    // <Name>Static fields should be prefixed with a 's_'</Name>
    WARN IF Count > 0 IN SELECT FIELDS WHERE 
     !NameLike "^s_" AND 
     IsStatic AND 
     !IsLiteral AND 
     !IsGeneratedByCompiler AND 
     !IsSpecialName AND 
     !IsEventDelegateObject 
    
    // This naming convention provokes debate.
    // Don't hesitate to customize the regex of 
    // NameLike to your preference.

    Although NDepend’s naming conventions do not quite fit my conventions, this rule is just plain awesome. I just had to edit the CQL to

    // <Name>Static fields should not exist...mostly</Name>
    WARN IF Count > 0 IN SELECT FIELDS WHERE 
     IsStatic AND 
     !IsLiteral AND 
     !IsGeneratedByCompiler AND 
     !IsSpecialName AND 
     !IsEventDelegateObject 
    
    // This naming convention provokes debate.
    // Don't hesitate to customize the regex of 
    // NameLike to your preference.

    and voilá – NDepend will now automatically find all static fields within my solution…and ignore any naming convention.

    Since this got me going, I also went ahead to modify the following rule

    // <Name>Instance fields should be prefixed with a 'm_'</Name>
    WARN IF Count > 0 IN SELECT FIELDS WHERE 
     !NameLike "^m_" AND 
     !IsStatic AND 
     !IsLiteral AND 
     !IsGeneratedByCompiler AND 
     !IsSpecialName AND 
     !IsEventDelegateObject 
    
    // This naming convention provokes debate.
    // Don't hesitate to customize the regex of 
    // NameLike to your preference.

    to instead require that fields are camel cased (ignoring the static condition as well):

    // <Name>Instance fields should be camelCase</Name>
    WARN IF Count > 0 IN SELECT FIELDS WHERE 
     !NameLike "^[a-z]" AND 
     !IsLiteral AND 
     !IsGeneratedByCompiler AND 
     !IsSpecialName AND 
     !IsEventDelegateObject

    Two small changes to the original setup, but awesomely helpful. Another great thing is that when you edit the queries in VisualNDepend, you get immediate, visual feedback to how the rule applies to the entire solution.

    So, now I can start tweaking the standard CQL rules to that they conform to my conventions. However, when looking at the two rules above, where my versions should apply to all future NDepend projects that I create from now on, is there some way to globally replace the standard CQLs with my alternatives?

    I will investigate this further and write a blog post if I happen to solve it.

     
    • Patrick Smacchia 2:31 pm on October 6, 2011 Permalink | Reply

      >However, when looking at the two rules above, where my versions should apply to all future NDepend projects that I create from now on, is there some way to globally replace the standard CQLs with my alternatives?

      There is no simple way yet to share a set of custom rules accross several projects.
      The options are:
      A) to copy/paste them accross NDepend running instances,
      B) to tweak the simple .ndproj project XML somehow to share the rules.

      We are aware of this limitation and are working on a feature that will make this easier.

      • danielsaidi 10:41 pm on October 10, 2011 Permalink | Reply

        Hi Patrick! I just wanted to let you know that today’s analysis worked great, without any problems whatsoever. My computer started up the bat script automatically, which checked out and built the latest source code, then ran an analys for each of the seven solutions…and finally posted the result on an internal server. Good stuff! 🙂

    • danielsaidi 3:44 pm on October 6, 2011 Permalink | Reply

      It is really not a problem – if the rule addresses some convention that the code does not share, one will get numerous warnings and can then decide whether or not the warnings are an issue or not. Also, I have modified the rules a bit differently in some projects, so maybe it is a good thing that one has to do these modifications for each project.

      I will gather my rules in simple text files that I keep under version control, together with my NDepend projects. I will probably also add the more general ones as resources to my .NExtra project.

      This is fun! 🙂

  • danielsaidi 2:57 pm on October 5, 2011 Permalink | Reply
    Tags: code analysis, , , system architecture, task scheduler   

    Scheduling NDepend for a set of solutions 

    In a project of mine, I use NDepend to continuously analyze a set of solutions that make up some of the the software infrastructure of a major Swedish company.

    By scheduling the analyses to run once a week, using previous analyses as a baseline for comparison, I hope that this will make it easier to detect less favorable patterns that we want to avoid and pin-point good ones that we want to embrace.

    Although we use Team City as build server, I have setup the scheduled analyses to run from my personal computer during this first test phase. It is not optimal, but for now it will do.

    The analyses are triggered from a simple bat script, that does the following:

    • It first checks out each solution from TFS
    • It then builds each solution with devenv
    • It then run a pre-created NDepend analysis for each solution
    • Each analysis is configured to publish the HTML report to a web server that is available for everyone within the project
    Once I had created the script, I scheduled it using the Task Scheduler.  I set it to run every Monday morning at 8.30. Since it runs from my personal computer, I have to be early at work, but with two kids at home, I always am 🙂

    The scheduled script works like a charm. The analyses runs each week and everyone is happy (at least I am). Already after the first analysis, we noticed some areas that we could modify to drastically improve the architecture, reduce branch/merge hell, code duplication etc.

    Who knows what we will find after some incremental analyses? It is exciting, to say the least!

    One small tweak

    During the experimentation phase, when the report generation sometimes did not work, I was rather annoyed when NDepend did not run a new analysis, since no code had changed. The solution was simple – under Tools/Options/Anaysis, tell NDepend to always run a full analysis:

    In most cases, though, the default setting is correct, since it will run a full analysis at least once per day. However, in this case, I keep the “Always Run Full Analysis” selected for all NDepend projects.

    One final, small problem – help needed!

    A small problem that still is an issue, is that my NDepend projects sometimes begin complaining that the solution DLL:s are invalid…although they are not. The last time this happened (after the major architectural changes), it did not matter if I deleted and re-added the DLL:s – the project still considered them to be invalid. I had to create the delete the NDepend projects and re-create them from scratch to make them work.

    Has anyone had the same problem, and any idea what this could be about? Why do the NDepend projects start complaining about newly built DLL:s?

     
    • Patrick Smacchia 4:23 pm on October 5, 2011 Permalink | Reply

      One remark: the incremental analysis option, is only valid in the standalone or VS addin context, no in the build server context of running an analysis through NDepend.Console.exe

      Next time it tells you an assembly is invalid, take your machine and shake it (but please stay polite with it)!

      If it still doesn’t work, go in the NDepend project Properties panel > Code to analyze > invalid assemblies should appear with a red icon, hovering an invalid assembly with the mouse will show you a tooltip that explains the problem (the problem will also be shown in the info panel).

      My bet is that several different versions of an invalid assembly are present in the set of the ndproj project dirs, where NDepend searches for assemblies (hence NDepend doesn’t know which version to choose).

      • danielsaidi 8:19 am on October 6, 2011 Permalink | Reply

        Kudos for your fast response, Patrick!

        Since I run the analyses locally, the incremental analysis option will apply if I do not disable it. However, the fact that ND will still run a full analysis at least once a day, means that I could enable the option once again, after the initial setup phase.

        I had a looong look at the failing assemblies prior to writing this post. ND complained about that multiple versions of some assemblies existed, even after I confirmed that the specified paths in fact did not. After I recreated the NDepend project, and re-added the assemblies – everything worked once again.

        I will have a look at how ND handles the assemblies next Monday, and let you know. I have an SSD in my machine, so I’ll first try to give it a rather rough shake 🙂

        Other than that, I am looking forward to start modifying the CQL rules now. I love the comment in the “Instance fields should be prefixed with a ‘m_'” rule! 🙂

    • Patrick Smacchia 8:46 am on October 6, 2011 Permalink | Reply

      >I will have a look at how ND handles the assemblies next Monday, and let you know

      Ok, sounds good, let us know

      >I love the comment in the “Instance fields should be prefixed with a ‘m_’” rule!

      🙂

c
Compose new post
j
Next post/Next comment
k
Previous post/Previous comment
r
Reply
e
Edit
o
Show/Hide comments
t
Go to top
l
Go to login
h
Show/Hide help
shift + esc
Cancel