I have a public method which searches for an image and if the image exists, it returns it as a byte array.
What the method should return, if the image doesn't exist? (the image not existing is not an exception, but expected possibility)
Should I return an empty byte[] array, or should it return byte?[] set to null instead?
Which one is more in tune with conventions?
I would return null (just as a byte[] - all arrays are reference types). Returning an empty array will lead the caller to think they can try to load that data as an image - which they can't. Returning a null reference makes it very easy to differentiate between "present and empty" vs "not present at all".
In other cases where the image being missing is an indication of a significant system problem, I'd consider an exception though. It sounds like that's not the case here.
Note that this is significantly different to a normal method returning a collection of individual elements - you don't care about each byte as a separate entity in the way that you would for a method returning List<Person> for example; here the byte[] is effectively a blob. I would regard it as a single chunk of data which happens to be represented as an array rather than a collection. To put it another way: think about it as if your method were declared to return a Blob type instead of an array... would you return a null reference, or an instance of Blob with no data?
if (img == null)?
LoadImageOrFail for example.catch block at a reasonably high level, but you should avoid throwing an exception just for it to be caught by the immediate caller - in most cases, anyway.Neither alternative is good for situations when something does not exist: returning an empty array of bytes is wrong, because the users cannot distinguish between finding an image that is actually empty, and not finding an image. Returning null is slightly better, but the users may forget to null-check the return.
Two alternatives that work better are returning a not found result as a bool while setting the result in an out parameter, and throwing an exception. Both approaches make the "image not found" condition explicit to the users of your API, making it harder to miss.
bool TryLookUpImage(string name, out byte[] image) {
...
}
...
byte[] image;
if (TryLookUpImage(imageName, out image)) {
// Display image
} else {
// Display error
}
TryFind ?It is considered a best practice to NEVER return null when returning a collection or enumerable. "ALWAYS" return an empty enumerable/collection. It prevents the aforementioned nonsense, and prevents your car getting egged by co-workers and users of your classes.
In general that's correct, but there are cases when returning null isn't wrong. so the "ALWAYS" has to be omitted.
Also, a great source is the Framework Design Guidelines 2nd Edition (pg. 256):
DO NOT return null values from collection properties or from methods returning collections. Return an empty collection or an empty array instead.
null if a searched resource does not exist. If you'd return an empty collection that would mean: here is your image, it is empty.Return a null since as @JonSkeet points out, since the result is not used as a collection but as a single blob. Maybe name the method GetImageOrNull so the user clearly knows he should expect to be given a null.
You can also do it like the Dictionary.TryGetValue method (link) and return a boolean that says if it's valid.
This could look like this (note that the method is named similar and - if used consistently - makes it obvious how to use it):
public bool TryGetImage(SomeParameter param, out byte[] image)
{
bool imageFound;
//...
return imageFound;
}
Change the return type to a IEnumerable<byte[]> instead. This could be interpreted as an array of images. Then you can return an empty byte[][]. Makes only sense if it's ever possible to return more than one image. This would also follow Atanas advice, which I second for collections.
IEnumerable<byte[]> if you try to align with any other API... Arrays are just pain and don't allow lazy evaluation...
ImageData and not an byte[]. Would make things even more readable... But I think that's out of scope for the current question.I would name the method
bool TryLoadImage(name, out byte[] image)
Then it is clear to anyone reading the code that the method expects not to be able to load some images. And therefore image can not be assumed to contain a valid image after the call.
Setting the image to null, rather than a empty array is best for all the reasons Jon Skeet gave.
byte[]reference which is null? No need to usebyte[].byte[]is reference type. You can do returnnulldirectly:byte[] Method() { return null; }.byte?[]means that you will create array of nullable bytes (so any / all of byte could be null, ) - that make no sense in this case.