Gendarme.Rules.Correctness.AvoidMethodsWithSideEffectsInConditionalCodeRule(git)

AvoidMethodsWithSideEffectsInConditionalCodeRule

Assembly: Gendarme.Rules.Correctness
Version: git

Description

A number of System methods are conditionally compiled on #defines. For example, System.Diagnostics.Trace::Assert is a no-op if TRACE is not defined and System.Diagnostics.Debug::Write is a no-op if DEBUG is not defined. When calling a conditionally compiled method care should be taken to avoid executing code which has visible side effects. The reason is that the state of the program should generally not depend on the value of a define. If it does it is all too easy to create code which, for example, works in DEBUG but fails or behaves differently in release. Methods which have no visible side effects are termed pure methods. Certain System methods and delegates (such as System.String and System.Predicate) are assumed to be pure. If you want to use a method which is not known to be pure in a conditionally compiled method call then you'll need to decorate it with PureAttribute (see the examples below). Note that it is OK to disable this rule for defects where the define will always be set (now and in the future), if the program's state really should be affected by the define (e.g. a LINUX define), or the impure method should be decorated with PureAttribute but it is not under your control.

Examples

Bad example:

using System.Collections.Generic;
using System.Linq;
using System.Runtime.Diagnostics;
internal sealed class Worker {
    private Dictionary<string, Job> jobs;
    private bool IsValid (Job job)
    {
        return job != null && job.PartId > 0;
    }
    private void Process (string name, Job job)
    {
        // This is OK because IsValid has no side effects (although the rule
        // won't know that unless we decorate IsValid with PureAttribute).
        Trace.Assert (IsValid (job), job + " is not valid");
        // This is potentially very bad: if release builds now (or in the future)
        // don't define TRACE the job won't be removed from our list in release.
        Trace.Assert (jobs.Remove (job), "couldn't find job " + name);
        job.Execute ();
    }
}

Good example:

using System.Collections.Generic;
using System.Linq;
using System.Runtime.Diagnostics;
// note: in FX4 (and later) this attribute is defined in System.Runtime.Diagnostics.Contracts
[Serializable]
[AttributeUsage (AttributeTargets.Method | AttributeTargets.Delegate, AllowMultiple = false)]
public sealed class PureAttribute : Attribute {
}
internal sealed class Worker {
    private Dictionary<string, Job> jobs;
    [Pure]
    private bool IsValid (Job job)
    {
        return job != null && job.PartId > 0;
    }
    private void Process (string name, Job job)
    {
        // IsValid is decorated with PureAttribute so the rule won't complain
        // when we use it within Trace.Assert.
        Trace.Assert (IsValid (job), job + " is not valid");
        bool removed = jobs.Remove (job);
        Trace.Assert (removed, "couldn't find job " + name);
        job.Execute ();
    }
}

Source code

You can browse the latest source code of this rule on github.com