Why FxCop is right – Enums should have zero value

Per FxCop – CA1008: Enums should have zero value

I ran a quick check on http://msdn.microsoft.com/en-us/library/ms182149.aspx.

An excerpt below:

The default value of an un-initialized enumeration, like other value types, is zero. An enumeration should define a member with the value of zero so that the default value is a valid value of the enumeration. If appropriate, name the member ‘None’. Otherwise, assign zero to the most commonly used member. Note that if the value of the first enumeration member is not set in the declaration, its value is zero by default.

Is it really worthwhile? I ran a quick check with the code below:

public enum TraceLevel
{
    Off = 0,
    Error = 1,
    Warning = 2,
    Info = 3,
    Verbose = 4
}

class Program
{
    static void Main(string[] args)
    {
        TraceLevel traceLevelOptions;
        Console.WriteLine(traceLevelOptions.ToString());
        Console.ReadLine();
    }
}

I thought FxCop missed something because I always got a compiler error when trying to use the default value of the enum without explicitly setting it to something first.

I couldn’t believe that FxCop could have a redundant rule. I quickly posted a question on Stackoverflow. Thanks to a quick response, I got the following answer to my question:

For example, you declare an enum field in a class or struct and do not initialize it, it will have the default value of zero. If there is no member of the enum with a zero value, you will in that (fairly common) situation have a enum field containing an invalid value.

There are also other instances where you can end up with a default-initialized enum field (e.g. during deserialization). You want to avoid a default-initialized enum field having an invalid value, hence the rule.

I then ran the following code and found it was indeed correct!

public class SerializeMe
{
    public int Id { get; set; }
    public TraceLevel MyTrace { get; set; }
}

public enum TraceLevel
{
    Off = 0,
    Error = 1,
    Warning = 2,
    Info = 3,
    Verbose = 4
}

class Program
{
    static void Main(string[] args)
    {
        SerializeMe serializeMe = new SerializeMe();
        Console.WriteLine(serializeMe.MyTrace.ToString());
        Console.ReadLine();
    }
}

Conclusion

If your enum does not have a zero value and you are not explicitly setting it to a specific value in your classes, using those classes for deserialization / serialization or json conversion or just accessing the property can lead to invalid values being used or accessed. 

 

Advertisements

How to review code

The first question we need to ask ourselves is what is the goal of our code review?

  • Report all code review issues?
  • Get all feedback incorporated before delivery?
  • Get the developers to follow the same paradigm and make the expected code quality a ritual in their development lives?

Your how should be in consonance with what is your goal. My personal view-point is that the last goal is more feasible than the first two and it will have a more profound impact on the code quality delivered over a long-term.

Till date, I have never seen an application where each and every deviation got reported in time. Despite the best of intentions at the time of delivery, we did find feedback that had to be scoped out too.

Due to these very reasons, the self-belief of the developers in the system will die out sooner than later. If mentally they are accepting that quality in code will not be met, and nothing can be done to avoid it, they will stop trying harder to deliver quality code.

To make good code quality a ritual for your developers you need to embrace the following practices in your team:

Establish a check list of items that will signify acceptable code quality for your team.

  1. Try not to include everything under the sun.
  2. Ideally the checklist should be something that can be printed in one A4 sheet

You may create periodic code quality campaigns in which your team is aware that you will be focussing on some specific code practices with zero tolerance

  1. This technique will be very helpful when you have a lot of pre-existing code that needs to be enhanced and your checklist looks too daunting for your developers as well as making the client ROI too challenging to achieve
  2. Some examples include ensuring all Web pages are compliant with a specific set of HTML standards, JS Lint standards, or, your stored procedures are created using static queries instead of dynamic queries
  3. Whatever campaign is chosen, it should be measurable for both the developers and (if applicable) for the clients.
  4. If the clients are able to appreciate, as in the case of performance boost as a result of static queries in place of dynamic queries, their feedback will encourage your developers even more to embrace the same standards
  5. If the effects are not that visible to your clients, then, it must make the lives of your developers easier. For example, when we targeted improvement in the HTML and JavaScript compliance, we were able to make our sites IE6 and IE8 compatible with lesser efforts and lesser issues reported.

Provide the tools to your developers for improving compliance with best practices. For example, our team has access to Resharper, JS Lint (integrated with Visual Studio), W3C Validator, etc.

  1. Tools like Resharper are great for boosting developer productivity on both the client and server sides

Review relentlessly and as early as possible

  1. Too many times, code is passed on to QA even before it is reviewed, primarily due to budgetary or timeline constraints. In most cases, the overall cost of a stable release is higher due to intermittent issues reported (especially when best coding practices are not part of the developer’s coding rituals)
  2. Once an issue is identified, ensure that it is incorporated by the concerned developer and by all the developers in your team
  3. The key is not how many issues you catch, but, each issue that you catch is stamped out.
  4. Subsequently, start maintaining a list of issues that you generally catch and look out for them in the code first.
  5. Also, build a case of the demerits of the issue and the benefits of the solution and discuss it with your developers
  6. The relentless review pertains to not ignoring the issues already reported in the past until the developer changes habit and begins coding the way expected first time around itself

Last, but not the least, choose your code champions

  1. Select people from the team who will champion the cause of best practices in a particular technology. It is better than expecting the same person to look in all aspects of the code including HTML, CSS, JavaScript, C#, SQL, etc.
  2. Give access to each code champion to do the following
    1.  Look at code in the specified technology across projects
    2. Share common mistakes in periodic lessons learnt meetings
    3. Propose white papers / presentations on how to achieve good quality code and increased productivity via re-usable code snippets / libraries or even suggestion of approaches

In conclusion, the more coding practices you inculcate in the developer’s coding ritual, the lesser time will be required in code review (in relation to those items). Although, code review will always be there as lessons will always be learnt, at least the team will be consistent and meet the standards that are made a focus area. This makes it easier for developers to switch project teams or move from one project to another without losing out on productivity.

Updated on November 20, 2012:

Based on some feedback received on Facebook from my dear friend, Moin, the following seem necessary to be added to this blog.

Further thoughts about raising code quality standards via code review

  • It is far easier moulding entry level programmers towards maintaining coding standards. However, the entry level programmers will always remain a fraction of the pie. A majority of the programmers will always be mid or senior level and many of them would have joined from different firms where the coding standards may differ.
  • For mid and senior level programmers, the same review process can be followed, but as a manager we must be ready to answer tough questions with assertiveness. I have seen this happen in my team and although at times, a bit of effort goes into explaining the rationale, generally, the team support is there
  • Code champions in this article refer to those people who will champion the appropriate usage of the technology, and, have the in-depth understanding to be able to explain their code review to the concerned programmer. Their focus will also be to pick out gems of code snippets and share it across teams as well as facilitate consistent style of programming
  • The code review should not be mixed with code audit. Code audit is done after a phase of coding has been completed. A code audit will definitely let you know what is missing in the code and a lit of issues will be available for the programmer to incorporate. However, it can also lead to programmers becoming defensive about their code. It is better to spend a bit of time with them while they are programming and provide on the spot feedback and focus on assisting the programmer to evolve rather than focus in issue reporting. It will also lessen the pressure on the programmer to incorporate feedback at the end of the coding cycle. A code audit at the end of the coding phase will still be useful in tracking how successful we are in the initiatives taking during coding.

Leadership Wisdom by Robin Sharma states that we all want appreciation and recognition. We tend to do more of what we get genuinely recognized and appreciated for. All thoughts above as well as suggestions are from the perspective of helping our programmers become recognized as success stories rather than being viewed as deficient in some aspect of coding.