6
\$\begingroup\$

I have a class as follows, where I want the user of the class to know that setting the Slug property to null is supported, and its getter will still return a valid value:

public class Post
{
    public string Title { get; }
    public Post(string title)
    {
        Title = title;
    }

    private string? slug;
    public string Slug 
    { 
        get
        {
            if(slug is null)
            {
                slug = Title.Slugify();
            }
            return slug;
        }
        set => slug = value;
    }
}

There are two possible cases here:

CASE 1: No slug is set on/after initialization OR the slug is initialized with a non-null string.

var post = new Post("foo bar");
var slug = post.Slug;               // foo-bar

var post2 = new Post("foo bar 2") { Slug = "foo-bar-2" };
var slug2 = post.Slug;              // foo-bar-2

CASE 2: The slug set on is initialization with a nullable string.

string? maybeNull = viewModel.Slug; // may or may not be null
var post = new Post("foo bar") { Slug = maybeNull! };
var slug = post.Slug;               // returns an expected value

Is there a better way to do this, than using the null forgiving operator? The above code looks worse if I had to set it to null:

var post = new Post("foo bar") { Slug = null! };
var slug = post.Slug;               // returns an expected value

The reason I dont want to use the null forgiving operator is because it requires the user of the Post class to know that a null value can be safely passed into its Slug property -- something I wish to avoid.

If it's not possible to have different nullability for getters and setters (appears to me like it's not), what is the best way to communicate my intent (users can safely set Slug to null + users will always get a valid slug, irrespective of what was set).

For some more context:

  • the Post class is instantiated with a nullable value only while creating a Post. Every other time, it's going to be pulled from a database, and so will always be instantiated with a valid slug.
  • the Post class is not a ORM model.
\$\endgroup\$
13
  • 2
    \$\begingroup\$ Did you think about adding a method "useTitleAsSlug", while leaving the setter to not accepts null? That way it may be more explicit. Anyway maybe the setter should check the null and getter should not modify the internals... \$\endgroup\$
    – slepic
    Aug 3, 2020 at 6:25
  • \$\begingroup\$ @gldraphael. In case of null if you can specify a fallback value then all need to do is to modify the setter of Slug like this: set => slug = !string.IsNullOrEmpty(value)? value : fallbackValue; \$\endgroup\$ Aug 3, 2020 at 6:50
  • \$\begingroup\$ @slepic I did consider adding a method, but was hoping to avoid that. (My goal is to communicate that setting a valid slug is optional, the user of the class will still get a valid slug.) If the setter checks the value, there's the getter might return null if the setter wasn't used. Lazy initializing a property in the getter is a common pattern in C#. Peter, that will still require me to use the null forgiving operator. \$\endgroup\$
    – galdin
    Aug 3, 2020 at 7:09
  • 1
    \$\begingroup\$ That file is different from what you posted, is your code supposed to replace it? As I suggested, please add what prompted you to write this to the question. It's part of an EventAggregate it seems, that's valuable information. \$\endgroup\$
    – Mast
    Aug 3, 2020 at 7:21
  • 1
    \$\begingroup\$ Unless I misunderstood, you are contradicting yourself: (1) "I want the user of the class to know that setting the Slug property to null is supported" (2) "it requires the user of the Post class to know that a null value can be safely passed into its Slug property -- something I wish to avoid" \$\endgroup\$
    – Flater
    Aug 3, 2020 at 9:39

3 Answers 3

4
\$\begingroup\$

I'm thinking you should probably change your Post class as follows:

public class Post
{
    public string Title { get; }
    public string Slug { get; set; }
    public Post(string title)
    {
        Title = title;
        Slug = title.Slugify();
    }
}

Now you have a slugified title by default. However, if a user of the Post class wants to provide an alternative slug, they can still do that:

var post = new Post("foo bar") { Slug = altSlug };

The explanation for the user should be that they can either provide a slug explicitly, or have one auto-created by omitting it from the initializer. The only thing you cannot do with this code is set Slug to null explicitly. But I think it would not be good design to allow that.

On a side note, you might even want to make the Slug property getter-only, creating an immutable class, and providing a second constructor that takes the alternative slug. This makes reasoning easier:

public class Post
{
    public string Title { get; }
    public string Slug { get; }
    public Post(string title) : Post(title, title.Slugify()) {}
    public Post(string title, string slug)
    {
        Title = title;
        Slug = slug;
    }
}
\$\endgroup\$
7
  • \$\begingroup\$ Thanks for the answer, can you please add the [AllowNull] to it? I considered making an immutable (PostMetadata) class too, perhaps with C# 9's record types that will feel more natural as well. \$\endgroup\$
    – galdin
    Aug 3, 2020 at 9:50
  • 1
    \$\begingroup\$ The thing is, if you take the design as suggested in this answer, you don't need the [AllowNull] attribute anymore. \$\endgroup\$
    – dumetrulo
    Aug 3, 2020 at 9:52
  • \$\begingroup\$ I agree, and on second thought what you suggest feels more clear too. \$\endgroup\$
    – galdin
    Aug 3, 2020 at 9:54
  • \$\begingroup\$ And if you find that you need settable properties for serialization or something, using get; private set; is probably the answer. \$\endgroup\$
    – dumetrulo
    Aug 3, 2020 at 9:56
  • \$\begingroup\$ How about a constructor forcing a non-null slug value, and no public setter for it. Besides an exception for passing null , a default value or perhaps the null object pattern could be a possibility. In any case the allow null but can't be null contradiction must be resolved. \$\endgroup\$
    – radarbob
    Aug 3, 2020 at 19:36
4
\$\begingroup\$

This answer is more philosophical than it is technical, as your question revolves around intent and communication, not technical implementation or syntax (directly). You have some expectations, but aren't quite taking them to their logical conclusion.

If you're just interested in the code, read the last section. The rest is explanation and justification as to why this is a better approach.


Your intentions are unclear

I want the user of the class to know that setting the Slug property to null is supported

... it requires the user of the Post class to know that a null value can be safely passed into its Slug property -- something I wish to avoid"

That's a contradiction.

The reason I'm bringing this up is because this question revolves around communication between developer and consumer, not technical implementation (directly). But if you want your developer and consumer to be on the same page, you need to first know what that page is supposed to be, and that contradiction isn't helping you.


The typical property

Generally, you expect a gettable and settable property to retain the state you set.

var myClass = new MyClass();
myClass.MyProperty = "hello";

Assert.AreEqual(myClass.MyProperty, "hello"); // Should pass!

That is the default expectation on how a property behaves. Is that behavior set in stone? No. Clearly, the language/compiler allows you to implement your properties completely differently.

But doing so means you go against the consumer's natural intuition on how a property behaves.

The conclusion I'm trying to get to here is that you need a valid reason to deviate from the default property behavior, if you want to justify the cognitive cost to your consumer, who now has to know about this atypical behavior in your implementation.

Sometimes, this can already be contextually understood, e.g. if your setter cleans up an understandable-but-abnormal value (e.g. a file path with consecutive separators). Sometimes, this requires documentation to explain to the consumer.

I have no idea what a Slug is, so I can't judge this part. Based on the root your question, it would seem that this needs to be explicitly communicated to the consumer.


The null crutch

Historically, null is a controversial topic. Some developers hate it, some use it religiously. I'm not making any definitive statements here. Let's avoid a holy war and try to find middle ground: null can have its purposes, but null can also be abused.

One clear abuse of null is when developers use it as an additional value which they don't bother to further define. The clearest example here is turning a bool into a bool? when a third state needs to be added. It should've been an enum with 3 members, but null is being introduced as the third value instead, as a shortcut.

What you're doing here is similar. Your Slug doesn't have "the chosen value or null", your Slug has "the chosen value or a default value". You've just chosen to represent that choice of setting the default value using null (and then subsequently translate a null into the actual default value you want), which in my book counts as the same null abuse I just described.


Solution: the named default

We've addressed several issues:

  • The way you're suggesting to use your property is atypical and would require the consumer to learn the specifics of your implementation
  • The way you're suggesting null should be used to set a (non-null) default value is atypical and would require the consumer to learn the specifics of your implementation.

This can't live in the same world as:

  • You want the code to be self-documenting towards the consumer.

If you stray from the beaten path, then people won't find their way on their own.

To solve the above issues, we should make the "default" value an explicit part of the contract of your Post class. This way, your consumer can figure out that there is a default value, and how to set it, without needing to read additional documentation.

The simplest solution here is to stick with a non-null property, and add a secondary route to set that property value.

public class Post
{
    public string Title { get; }
    public string Slug { get; set; }

    public Post(string title)
    {
        Title = title;
        SetDefaultSlug();
    }

    public void SetDefaultSlug()
    {
        Slug = title.Slugify();
    }
}

The main difference between this answer and the already given answer by dumetrulo is that this version can revert back to the default, whereas the other answer's default, once removed, cannot be retrieved (since your consumer doesn't know how you calculate the default value.


Additionally, you can argue that you should still use the default value when the custom value doesn't pass a certain validation (e.g. no empty strings).
I can see arguments pro and con, depending on whether you consider this a valid responsibility of your class. That's a contextual consideration which I cannot conclusively judge. Should you judge it to be relevant, you can change your Slug property to:

private string _slug;
public string Slug
{
    get
    {
        return ValidateSlug()
                 ? _slug
                 : title.Slugify();
    }
    set { _slug = value; }

}

private bool ValidateSlug()
{
    return !String.IsNullOrWhitespace(_slug);
}

A null or empty check is just a basic example. This could be based on length, profanity filter, ... Your business validation is your own decision.

However, do keep in mind that we're getting into atypical property behavior again. If your Post's responsibility includes sanitizing the slug (which would be the basis for adding the above code), then that's a valid justification for changing the property's default behavior.

But that depends on the notion that the consumer inherently understands that the Post class will sanitize the slug as it sees fit.

\$\endgroup\$
4
  • \$\begingroup\$ Thank you for your well thought out answer. A slug is a url segment. There's no contraction: the first statement is what I want, the second statement is what I have (but dont want). I agree with the typical property section -- that is what made me feel like I was asking for the wrong thing in the first place. The other answer suggested to make the class immutable, i.e. don't change but replace. \$\endgroup\$
    – galdin
    Aug 3, 2020 at 10:54
  • \$\begingroup\$ @gldraphael: I'm aware I may be misreading, but the second statement explicitly expresses "what you want to avoid", i.e. you want to avoid the user being required to know of the null feature; which is the opposite of the first statement. Either I'm misreading or you misspoke? \$\endgroup\$
    – Flater
    Aug 3, 2020 at 11:01
  • 2
    \$\begingroup\$ @gldraphael: Immutability does make things easier as your object's state isn't in flux. But not every class can be made immutable without consequences - and I cannot judge that context. If immutability works for you, it is definitely worth considering as it simplifies your problem scenario. \$\endgroup\$
    – Flater
    Aug 3, 2020 at 11:03
  • \$\begingroup\$ aah now i see what you mean about the contradiction, my bad 🤦‍♂️. I'll leave it unedited. \$\endgroup\$
    – galdin
    Aug 3, 2020 at 11:29
3
\$\begingroup\$

[AllowNull] does seem to be exactly what you need. If slug was declared as:

    private string? slug;
    [AllowNull]
    public string Slug 
    { 
        get
        {
            if(slug is null)
            {
                slug = Title.Slugify();
            }
            return slug;
        }
        set => slug = value;
    }

One of the answers suggest that allowing a property to have a non-nullable getter, but nullable setter is "atypical" and straying off the beaten path. But it's not quite true. Let's look at the UX, when using the [AllowNull]:

// Creating a Post without setting a slug
Post post1 = new Post(title: "Foo bar");
string slug1 = post1.Slug; // slug = "foo-bar"

// Creating a Post by setting an explicit slug
Post post2 = new Post(title: "Foo bar 2) { Slug = "hello-world" };
string slug2 = post2.Slug; // slug = "hello-world"

// Creating a Post by setting an explicit nullable slug
string? slug = vm.Slug;    // maybe null
Post post3 = new Post(title: "Foo bar 3") { Slug = slug }; // note that we dont need the ! operator
string slug3 = post3.Slug; // slug3 = slug

No compiler warnings, no null forgiving operators. The user can set the property if they want to, or set it to a nullable value. But when they get the value, since the value will not be null (implementation detail), so the compiler won't ask to check for null (what the API user sees).

From the documentation:

you can apply a number of attributes: AllowNull, DisallowNull, MaybeNull, NotNull, NotNullWhen, MaybeNullWhen, and NotNullIfNotNull to completely describe the null states of argument and return values. That provides a great experience as you write code. You get warnings if a non-nullable variable might be set to null. You get warnings if a nullable variable isn't null-checked before you dereference it.

The other answers are spot-on about using a fully immutable type, and IMO is the best approach. Perhaps (if Post is an Entity) this could also be an opportunity to extract properties into a ValueObject.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.