search

dotnet / spark

.NET for Apache® Spark™ makes Apache Spark™ easily accessible to .NET developers.
https://dot.net/spark
MIT License
2.02k stars 312 forks source link

[C#] For public APIs, check nulls for parameters. #22

Open imback82 opened 5 years ago

imback82 commented 5 years ago

For example,

public DataFrameWriter Format(string source)

source should be checked if it is null before sending it to JVM side.

One thing to consider is whether we want to check it at the API level, or at the JVMBridge level.

Checking at the API level will introduce lots of code, but will be better for the user since the exception stack will be smaller. Checking at the JVMBridge level will be much less code, but user will see the exception stack from the JVMBridge.

imback82 commented 5 years ago

@stephentoub @rapoth Any thoughts here? If we do check it at the API level, we will be updating potentially hundreds of functions.

stephentoub commented 5 years ago

If this happens in C# 8 (https://github.com/dotnet/csharplang/issues/2145, https://github.com/dotnet/csharplang/blob/master/meetings/2019/LDM-2019-01-14.md), it'd definitely be better to do at the call sites, as that will not only be simple but also self-documenting.

Until then, I'd still suggest it'd be best to do at the call sites, but if it's simpler to start doing it by putting them check in some common place, that's ok. I'm less worried about the resulting call stack in exceptions and more concerned with how obvious these things are for someone reading/maintaining the code.