0

I've enabled the C# 8.0 non-nullable reference types feature in one of my projects, but now I'm unclear about how to represent missing data.

For example, I'm reading a file whose lines are colon-separated key/value pairs. Sometimes there's more than one colon on a line. In that case, the text before the first colon is the key, and the rest is the value. My code to parse each line looks like this:

public (string key, string value) GetKeyValue(string line)
{
    var split = line.Split(':');
    if (split.Length == 2)
        return (split[0].Trim(), split[1].Trim());
    else if (split.Length > 2)
    {
        var joined = string.Join(":", split.ToList().Skip(1));
        return (split[0].Trim(), joined.Trim());
    }
    else
    {
        Debug.Print($"Couldn't parse this into key/value: {line}");
        return (null, null);
    }
}

What this does: If we have just one colon, return the key and value. If we have more than one, join the rest of the text after the first colon, then return the key and value. Otherwise we have no colons and can't parse it, so return a null tuple. (Let's assume this last case can reasonably happen; I can't just throw and call it a bad file.)

Obviously that last line gets a nullability warning unless I change the declaration to

public (string? key, string? value) GetKeyValue(string line)

Now in F# I would just use an Option type and in the no-colon case, I'd return None.

But C# doesn't have an Option type. I could return ("", ""), but to me that doesn't seem better than nulls.

In a case like this, what's a good way to say "I didn't find anything" without using nulls?

Ryan Lundy
  • 204,559
  • 37
  • 180
  • 211
  • 3
    What is the problem with using `string?` ? Also, if you think that such an Option type would be the best solution, can't you add one? – Lasse V. Karlsen May 15 '19 at 08:50
  • If it's nullable, then make it nullable right? – DavidG May 15 '19 at 08:51
  • 2
    Be aware that non-nullable reference types doesn't mean you need to ban nullable reference types, it just means you have a new/different/better tool to make sure you manage the risk of null reference exceptions. – Lasse V. Karlsen May 15 '19 at 08:51
  • Before I either just make the method nullable or use empty strings instead, I'm wondering whether there's a better alternative. Similar to how there _is_ a better alternative in F#. – Ryan Lundy May 15 '19 at 08:59
  • Since it is a search of sorts, perhaps return an array with 0 elements instead of null, 1 element if the key is found, and `n` elements if the key is found multiple times. – John Wu May 15 '19 at 08:59
  • Adding `?` in function return declaration will sove the issue. Raising other warning of codes not handeling the nullable. From the beginning the code intend to use nullable but it was not explicit. What is the meanning of those null. May this meaning be carry by an other property type tht will be more explicite that the newly enforce `T?` – xdtTransform May 15 '19 at 09:14

3 Answers3

0

You could include if the result was successful in parsing by just returning a flag:

public class Result
{
    private Result(){}

    public bool Successful {get;private set;} = false;

    public string Key {get; private set;} = string.Empty;

    public string Value {get; private set;} = string.Empty;

    public static Successful(string key, string value)
    {
        return new Result
        {
            Successful = true,
            Key = key,
            Value = value
        };
    }

    public static Failed()
    {
        return new Result();
    }
}

public Result GetKeyValue(string line){
     return Result.Failed();
}

Then you could use it like

var result = GetKeyValue("yoda");

if(result.Successful)
{
    // do something...
}

Alternatiely you could return 2 diffrent types and use pattern matching

Kevin Smith
  • 13,746
  • 4
  • 52
  • 77
0

Actually, I realize now that part of the problem is that my method is doing two separate things:

  • Determine whether the line has a key.
  • Return the key and value.

Thus the return value has to indicate both whether there's a key and value, and what the key and value are.

I can simplify by doing the first item separately:

bool HasKey(string line)
{
    var split = line.Split(':');
    return split.Length >= 2;
}

Then in the method I posted, if there's no key, I can throw and say that the lines need to be filtered by HasKey first.

Ryan Lundy
  • 204,559
  • 37
  • 180
  • 211
0

Putting on my functional thinking cap, an idiomatic return type would be IEnumerable<(string?,string?)>. The only change to your code would be to change return to yield return, and to remove the return statement if nothing is found.

public IEnumerable<(string? key, string? value)> GetKeyValue(string line)
{
    var split = line.Split(':');
    if (split.Length == 2)
        return (split[0].Trim(), split[1].Trim());
    else if (split.Length > 2)
    {
        var joined = string.Join(":", split.ToList().Skip(1));
        yield return (split[0].Trim(), joined.Trim());
    }
    else
    {
        Debug.Print($"Couldn't parse this into key/value: {line}");
    }
}

The caller then has several options on how to handle the response.

If they want to check if the key was found the old-fashioned eway, do this:

var result = GetKeyValue(line).SingleOrDefault();
if (!result.HasValue) HandleKeyNotFound();

If they prefer to throw an exception if the key is not found, they'd do this:

var result = GetKeyValue(line).Single();

If they just want to be quiet about it they can use ForEach, which will use the key and value if they are found and simply do nothing if they are not:

foreach (var result in GetKeyValue(line)) DoSomething(result.Item1, result.Item2);

Also, for what it's worth, I'd suggest using KeyValuePair instead of a tuple, since it clearly communicates the purpose of the fields.

John Wu
  • 50,556
  • 8
  • 44
  • 80