Should I assign a default value for this property in the constructor?

Question

I'm currently creating a class in C# that will be utilized to help standardize calls to restful web services in my company's code. While the majority of my code has been written and well tested, I believe I made a mistake by assigning a default value to one of my properties in the default constructor.

The property in question is called Method and is responsible for setting the HTTP Method of the HttpWebRequest object. Currently, we only support GET and POST and do not have plans to implement other methods such as PUT, DELETE, etc.

private string method = "GET";
public string Method {
    get { return method; }
    set {
        if (value.ToUpper() != "GET" && value.ToUpper() != "POST")
            throw new ArgumentException($"The supplied method '{value}' is not supported. Please use 'GET' or 'POST' instead.");

        method = value.ToUpper();
    }
}
public RestfulService() {
    //...
    Method = "GET";
    //...
}
public RestfulService(string name, string uri) : this() {
    //...
}
public RestfulService(string name, string uri, string method) : this(name, uri) {
    Method = method;
}

I believe that I should be setting the Method property to string.Empty by default, and if the user doesn't supply the Method then the service won't execute (I.E. I'll check for that, and throw an exception if they didn't supply it). However, I like the idea of using a supported method by default.

---

According to best practice and common expectation when working with frameworks, should I set the default value for Method to string.Empty or a supported HTTP method?

NOTE: Please ignore any compilation errors the supplied block of code may have (syntax, spelling, etc). I simply typed up what I believed to be relevant to this post.

Answer 1

Why allow the impossible?

I believe that I should be setting the Method property to string.Empty by default, and if the user doesn't supply the Method then the service won't execute (I.E. I'll check for that, and throw an exception if they didn't supply it).

Why? Why would you create a parameterless constructor if you know that it's going to blow up during runtime? What is the purpose of making your consumer (developer) think that they are allowed to use a parameterless constructor, have it compile, only to then invariably never work at runtime?

I do notice that the property can be set after the object has been initialized, but this makes it unclear to the consumer whether the method is actually required or optional.
You clearly want to enforce that the method gets set at some point, and the way to implement this is to enforce it as a constructor parameter. The nice thing about constructors is that you must comply with (one of) them, and if you don't you don't even get an instantiated object. This is a really good way to enfore that your consumer provides the information you consider to be required.

By explicitly opening the door to initializing the object without setting the method, you are communicating to your consumer "you don't need a method to have a RestfulService, it's optional", which it isn't.

Simply remove the parameterless constructor. That way, your consumers are forced to state exactly which method they intend to use. If a developer refuses to pass the value into the constructor, then their code will never compile, which a clear explanation as to why they can't do what they're trying to do.

Sure, they can still explicitly pass String.Empty (or any other invalid value) as a constructor parameter, but we will tackle this issue later on in the answer.

---

Default values make sense when they fullfil default expectations

public RestfulService() 
{
   //...
   Method = "GET";
   //...
}

However, I like the idea of using a supported method by default.

Why is GET special? Why did you choose this to be the default? Is there reason to suspect that your consumers will innately assume that GET is the default?

Default values only make sense in cases where consumers didn't need to think about the value and the default value is commonly understood to be the default expectation of the consumer.

For example, myString.Substring(1) omits the second parameter (the ending index of your substring) because we can infer that since the consumer didn't feel the need to specify it, it doesn't matter to them and therefore the substring goes to the end of the source string.

Think of it in semantical English terms:

• myString.Substring(1,3) = "Give me the substring from the second to the fourth character"
• myString.Substring(1) = "Give me the substring from the second character"

In the second case, "until the last character" is implicitly understood (in common English) and therefore it makes sense to make the end index parameter's default value to be the end of the string.

But if you require your consumers to know that you arbitrarily decided that the default value is "GET", and this isn't an implicit understanding, then you're running into a problem. Before the consumer can decide to omit an explicit value, they need to first go and find out what the default value is, and then you're violating your encapsulation by forcing the consumer to look inside your RestfulService class.

To be fair, there is a reasonable argument to be made here for GET being the default method, but I'm trying to get you to understand the reasoning why default values should(n't) be used. While it makes sense to have GET be the default here, the same conclusion may not hold true for other default values you'll think of implementing in the future.
That being said, it's not wrong either to still disagree and instead require your consumer to be explicit about the method. Overly liberal use of default values makes codebases harder to read and it's not unreasonable to want to avoid default values because of it. This is a matter of style, approach, and consumer expectation.

---

GET WET

private string method = "GET";

public RestfulService() 
{
    //...
    Method = "GET";
    //...
}

Just a small sidenote, you're setting the default value twice. There's no point to repeating yourself. Either set the default value in the constructor or in the field.

---

Enums vs magic strings

if (value.ToUpper() != "GET" && value.ToUpper() != "POST")

Since there are only two possible valid methods, it's bad practice to have the user use magic strings. Instead, you should implement a simple enum so that the user doesn't need to worry about typo's and instead can rely on clear Intellisense.

It also means you don't have to validate the passed value anymore.

Fringe comment:
It's technically possible for the consumer to use a different enum value by forcing a different int value into the enum, but this is explicit bad handling from the consumer's side and unexpected behavior from your RestfulService is allowed. It's pointless to try and defend from willful mistakes on the consumer's part, unless there are stringent security considerations, which there aren't here.

---

Adjusting your code example

My suggestion is as follows:

• Either use a publicly settable property or a constructor parameter, not both. It not that it can't be done from a technical, but it makes it unclear for the consumer as to how they need to set the needed values.
• The method value is not optional, it is paramount for the RestfulService to perform its duty. To that end, it makes sense to have this as a constructor parameter, especially given that the URL is already being set in stone in the constructor anyway
• There is a reasonable argument for making GET the default method, so I'll implement it.
• There is no reasonable argument for having a parameterless constructor as instantiating a RestfulService without a method or URL is pointless, so the parameterless constructor goes away.
• I will use an enum to provide a closed list of valid methods.

public class RestfulService
{
    public enum HttpMethod { GET, POST }

    public HttpMethod Method { get; private set; }

    public RestfulService(string name, string uri) 
    {
        this.Method = HttpMethod.GET; //default value

        //also set the name and URI
    }

    public RestfulService(string name, string uri, HttpMethod method) : this(name, uri) 
    {
         this.Method = method; //override the default value
    }
}

This is all you need to set it up properly. If the consumer does something you don't want them to, it generally results in compile-time (as opposed to runtime) errors.

Answer 2

There is nothing wrong with having sensible default for values. Only your team can judge if having a default is sensible in this specific case. If there is a default value it should be clearly documented, see 'principle of least astonishment'.

Another principle is that when the object has been constructed it should be in an valid state. Your suggestion to leave the 'method' empty and fail when used would violate this. If you remove the default value you should take the 'method' as a parameter in all constructors to ensure it is set. If any validation of parameters is needed, it should be done when the constructor is called.

As mentioned in the comments, you should also used a strong type for the method parameter. If you only allow 'get' and 'post' a custom enum would seem appropriate. This should guarantee that all usable parameter values are valid.