What are your organization's expectations or policies regarding PR size and acceptable AI usage? Even if your organization hasn't set any expectations, what are yours—and have you communicated them to the author?
If expectations have been shared and these changes contradict them, you can quickly close the PR, explain why it's not acceptable, and ask them to redo it.
If you don't have clear guidelines on AI usage or haven't shared your expectations, you'll need to review the PR more carefully. First, verify whether your assumption that it’s a simple service is accurate (although from your description, it sounds like it is). If it is, talk to the author and point out that it's more complicated than necessary. You can also ask if they used AI and warn them about the complexities it can introduce.
Thanks for the upvotes! While testing and writing about the feature, I suspected it would receive mixed reactions.
The `?.` operator behaves similarly on the LHS to the RHS, making the language more consistent, which is always a good thing. In terms of readability, I would say that once you understand how the operator works (which is intuitive because the language already supports it on the RHS), it becomes more readable than wrapping conditionals in `if` statements.
There are downsides, such as the overuse I mentioned. But this is true for many other language features: it requires experience to know when to use a feature appropriately, rather than applying it everywhere.
However, the great thing about this particular enhancement is that it's mostly cosmetic. Nothing prevents teams from not adopting it; the old syntax still works and can be enforced. C# and .NET are incredibly versatile, which means code can look dramatically different depending on its context and domain. For some projects, this feature might not be needed at all. But many codebases do end up with various conditional assignments, and in those cases, this can be useful.
I have long desired such a language feature. It is a great addition to the language, because a single such expression helps to avoid potential bugs caused by mismatching double references in the conditional test and the execution statement of the traditional version, especially when refactoring longer code blocks.
For example, if we have something like this:
if (config?.Settings is not null)
{
... Multiple lines that modify other settings.
config.Settings.RetryPolicy = new ExponentialBackoffRetryPolicy();
}
and we introduce another category SpecialSettings, we need to split one code block into two and manually place each line in the correct code block:
if (config?.Settings is not null)
{
... Multiple lines that modify other (normal) settings.
}
if (config?.SpecialSettings is not null)
{
... Multiple lines that modify other special settings.
config.SpecialSettings.RetryPolicy = new ExponentialBackoffRetryPolicy();
}
With the new language feature the modification is easy and concise:
config.Settings?.RetryPolicy = new ExponentialBackoffRetryPolicy();
becomes:
config.SpecialSettings?.RetryPolicy = new ExponentialBackoffRetryPolicy();
and can be made for any other special setting in place, without the need to group them.
Furthermore, I find the "Don't Overuse It" section of the article somewhat misleading. All the issues mentioned with regard to
if (customer is not null)
{
if (customer.Orders is not null)
{
if (customer.Orders.FirstOrDefault() is not null)
{
customer.Orders.FirstOrDefault().OrderNumber = GenerateNewOrderNumber();
}
}
}
or:
if (customer is not null)
{
var orders = customer.Orders;
if (orders is not null)
{
var firstOrder = customer.Orders.FirstOrDefault();
if (firstOrder is not null)
{
firstOrder.OrderNumber = GenerateNewOrderNumber();
}
}
}
If it really were a bug, when customer is null here, etc., then it would of course make sense to guard the code as detailed as described in the article. However, this is not a specific issue of the new language feature. Or to put it more bluntly:
I recently wanted to embed a draw.io diagram in Notion but couldn’t get it to work with the default Export as URL option in draw.io as it doesn’t load correctly in Notion.
This led me to build this super simple HTML page that I can use to specify the draw.io link and then get a link that can be embedded in Notion.
So far it’s worked really well for me so I thought I’d polish it up and share it with everyone in case you maybe find it useful!
If expectations have been shared and these changes contradict them, you can quickly close the PR, explain why it's not acceptable, and ask them to redo it.
If you don't have clear guidelines on AI usage or haven't shared your expectations, you'll need to review the PR more carefully. First, verify whether your assumption that it’s a simple service is accurate (although from your description, it sounds like it is). If it is, talk to the author and point out that it's more complicated than necessary. You can also ask if they used AI and warn them about the complexities it can introduce.
reply