CODEWITHSUNDEEP
c#.netcode-contracts
user3698624
user3698624

Reputation: 225

C# Usage of Redundant Code Contract-Preconditions

I did not find the info by reading the usage guidelines, so I ask my question here: Assuming the following situation:

    /// <summary>
    /// Initalisiert eine neue Instanz des SettingsManager
    /// </summary>
    /// <param name="settingsRepo">Das SettingsRepository welches die Einstellungen enthält</param>
    public SettingsManager(ISettingsRepository settingsRepo)
    {
        Contract.Requires<ArgumentNullException>(settingsRepo != null, "The parameter settingsRepo cant be null");

        _settingsRepo = settingsRepo;
    }

    /// <summary>
    /// Lädt alle Einstellungen
    /// </summary>
    public void Load()
    {
        Contract.Requires<NullReferenceException>(_settingsRepo != null, "The repository cant be null");
        .
        .
        .
    }

where _settingsRepo is a global private field of the class SettingsManager. In the constructor, I define the precondition, that the parameter settingsRepo is not null. So when settingsRepo equals null an Exception is thrown. But the method Load() uses the field _settingsRepo which definetly wasn't null during instantiation. However, I do not know if _settingsRepo equals null at the moment, when Load() is being used, so is the precondition I defined in the method Load() considered as redundant, or should I rest this.

Upvotes: 1

Views: 434

Answers (2)

Old Fox
Old Fox

Reputation: 8725

As I mention in the question's comment. You can remove the validation from Load method. The pattern you are using called Constructor Injection. The idea behind C'tor injection is the object you are trying to create needs a dependency and don't have any default/alternative behaviour if you don't supply the dependency. In your case it seem exactly the situation.

Summery: You verified the _settingsRepo is not null in the C'tor, therefore you don't need to verify it again in the Load' method.

This is a very good book about DI, I recommends you to read it.

Upvotes: 2

Rami Alshareef
Rami Alshareef

Reputation: 7150

Although you dont have a parameter passed to the method it worth to mention that the general rule is to validate arguments of public methods.

All reference arguments that are passed to externally visible methods should be checked against null. If appropriate, throw a ArgumentNullException when the argument is null.

If a method can be called from an unknown assembly because it is declared public or protected, you should validate all parameters of the method. If the method is designed to be called only by known assemblies, you should make the method internal and apply the InternalsVisibleToAttribute attribute to the assembly that contains the method.

This goes under the Code analysis : CA1062: Validate arguments of public methods

But since you have already validated _settingsRepo in a different place that is essential for the Load function to be exist, then you could omit the validation. But this might trigger a warning upon release, my suggestion is to run Code analysis prior to releasing the code

Read more

Upvotes: 0

Related Questions