How to prevent a System.Nullreferenceexception within an if

Asked

Viewed 323 times

-2

I’m developing a web application where a photographer does upload of an image. After uploading the image, I will scan the entire Metadata in order to capture some information from directories (Exif, IPTC, XMP, JFIF, PNG, BMP, JPG, etc.), I am using the library Metadata Extractor. Ok, I learned to use it. First I need to receive the directories:

 IEnumerable<MetadataExtractor.Directory> directories = iptcReader.ReturnMetadataFile(Path.Combine(StorageRootPOOL, imagePool.File_name[i]));

Here I already have all the directories of my image, after that I need to declare variables of each directory I need to capture in my case:

var IPTCDirectory = directories.OfType<IptcDirectory>().FirstOrDefault();
var JPEGDirectory = directories.OfType<JpegDirectory>().FirstOrDefault();
var JFIFDirectory = directories.OfType<JfifDirectory>().FirstOrDefault();
var EXIFDirectory = directories.OfType<ExifIfd0Directory>().FirstOrDefault();
var EXIFSubDirectory = directories.OfType<ExifSubIfdDirectory>().FirstOrDefault();

All values of directories ARE NOT NULL even if there is no directory in the image, the problem is in the tags that exist within them. To capture the tags inside the directories I use the following code:

imageFileInfo.Keywords = IPTCDirectory.GetDescription(IptcDirectory.TagKeywords);

imageFileInfo is the class where I will be storing all the data. My problem is, if for example there is no TAG IptcDirectory.TagKeywords I will receive a Nullexception, so far nothing out of the ordinary, but if I will prevent this error with an if for example:

 if (IPTCDirectory.GetDescription(IptcDirectory.TagKeywords) == null)
 {
 }

I get another one NullException, there’s the problem, I wanted to set a default value for tags that don’t exist, for example:

if (IPTCDirectory.GetDescription(IptcDirectory.TagKeywords) == null)
{
      imageFileInfo.Keywords = "valor padrão";
}

Remembering that it is no use I use the following code, because I need to know which tags are null and not which directories, because directories always exist, but some tags nay:

if (EXIFDirectory != null && ExifIfd0Directory != null) {
    //Os diretórios nunca são nulos e sim as tags, então esse exemplo não iria se aplica ao meu problema.
}

3 answers

7


Do not under any circumstances what is in the other answer by swallowing the exception. This does not solve any problem, it only worsens the situation. This is even more terrible than capture Exception. And use goto is even worse.

Only catch an exception if you really need to. She is slow and should only be used in exceptional situations, not for flow control.

If you’re getting one NullReferenceException, There’s a programming error there. You have to fix it. Capturing and swallowing the exceptions is not making the code right, the solution is to fix it.

Check with a if which can be null and handles the form you want, if you want to assign a default value, do this. So you may not need if.

You have to see what is generating the exception and check exactly this before the error happens. Let it be very clear whenever you receive NullReferenceException is a programming error that must be solved, it must not be captured. And checking the error before it occurs is the solution. That is why I still believe that the answer that has now been accepted is not appropriate, even if it is not the absurdity that the other proposed.

Maybe that’s what you want:

for (int i = 0; i < numberOfProcessAvailable; i++) {
    var IptcDirectory = directories.OfType<IptcDirectoryBase>().FirstOrDefault();
    imageFileInfo.Keywords = IptcDirectory.TagKeywords == null ? "padrao keywords" : IptcDirectory.GetDescription(IptcDirectory.TagKeywords);
    imageFileInfo.LensModel = IptcDirectory.TagLensModel == null ? "padrao lensmodel" : IptcDirectory.GetDescription(IptcDirectory.TagLensModel);
    imageFileInfo.TagMake = IIptcDirectory.TagMake == null ? "padrao make" : IptcDirectory.GetDescription(IptcDirectory.TagMake);
}

I put in the Github for future reference.

Read this: How to best treat exceptions in Java?. Even if it’s another language, it’s worth the same.

  • Bigown, alright, I will review what is correct or not, but the problem in this case is that there are certain tags for example from EXIFDirectory that it doesn’t exist, and I fall into that problem you understand? Has image that has and image that does not have, for those that do not have for example the tag "Datetimeoriginal" imageFileInfo, Do you have any idea how I can resolve this without using the treatment cited as wrong?

  • What I mean is, the directory itself is never null, but some tags inside it are, and I can’t deal with them with if because within the if expression I get an Exception null.

  • None of this is in your question, if you said the default value I would put in my code. I gave a base by not processing the item that is null. Your if is completely wrong, mine is probably right, just do not guarantee because the question has no details, there is no way I test. Fix the if instead of using the wrong mechanism.

  • I am reformulating the question more clearly, I thank you already (really), I prefer to follow the more laborious and correct side :)

  • 1

    There is nothing more laborious, unless you are referring to the work of learning to do the right, which I strongly recommend, exception is almost never the solution, most programmers do not know this, most believe it is good practice to make exception at all, what I already question the idea of good practice (https://answall.com/search?q=user%3A101+boas+pr%C3%A1ticas), the more that is good.

  • I agree Bigown, I even questioned the guy with the "right" question, but I’ve corrected my mistake. I rephrased the question, please see if it was clearer, your answer did not solve my problem (perhaps because I had not made clear the problem).

  • 1

    You made a nice change to the question, not only added details. There are new things that didn’t exist before and even what you had disappeared. I’m thinking of closing as unclear, because it’s getting more confusing, maybe the problem is the confusion.

  • There is no Getproperty feature in this library: github.com/drewnoakes/Metadata-extractor

  • I changed it because the other answer used it and you accepted it. So I thought that was correct in your code. If you accept an answer that doesn’t work, you are creating wrong information for the community. Was the old way (https://answall.com/revisions/221991/5) I did it right? I changed because I thought I had done the right thing, but the other acceptance made me see that I was wrong.

  • I really confused the balls, sorry, the old one doesn’t work either, because I fall into that IF that returns me null. For now it only works with Try catch, I’m going back because I don’t want you to have the wrong answer for the community.

  • I don’t fall for if some because there is no if in my reply. If none of this works, I think it’s best to close the question because you can’t provide enough information to help. You’re making a lot of trouble and that’s why you don’t have an answer to help you.

  • Dude, I made it clear in the question issue, including, your answer as I already said 300x does not solve, that comparison == null activates the nullException exception as I have already said.

  • is the only correct solution, if it does not solve, or lack information in the question, or is doing wrong.

  • No, it’s not the right solution, if it returns an error because it would be correct? I don’t want to be rude, but it’s just not correct. I’ll even try it again and post the print.

  • Okay, so if you know more than me, do as you please. Everyone who knows how to program in C# knows that this is the correct solution, that using exception is not correct. If it doesn’t work for you, it’s because your question doesn’t tell you correctly what you’re doing or implementing it wrong. When you realize that you will have the right solution, if you do not want to do it this way, do it the wrong way, it is your code that will get bad and it is your right to do it wrong. Right and works are very different things, what works one day has problems because it was not the right one. Every programmer should know this.

  • That’s the code, well-voted for who wants to learn to do right. That may not work s because the question does not clearly say where the error is.

  • Bigown, I didn’t say that an if is better than a Try catch, or something like, I said that that stretch you passed, is not correct, it returns me the same error when I do if (IPTCDirectory.GetDescription(IptcDirectory.TagKeywords) == null)&#xA; {&#xA; } You understand it a million times more than I do, I’m just saying it’s not solved with that answer.

  • Because the question is not clear, so I closed it, if it was clear I would put the code that prevents it.

  • If that’s not a clear question, I don’t know what it is anymore... but beauty.

  • If you don’t have the information to give you a proper response, it’s not clear, but you don’t want to hear it so beauty, the only one who’s hurt is you, only you won’t have the proper code for what you need. I answered what was asked, if you need something that is not in the question has no way to answer.

Show 15 more comments

1

I had understood your question the first time in a different way.

In your case the reason for NullReferenceException is clear-cut and acquaintance by you and is an expected behavior when a tag does not exist. In this case it is in your interest to capture the exception to assign a default value to tags nonexistent.

There is no problem in this case in you use:

try {
    imageFileInfo.Keywords = IPTCDirectory.GetDescription(IptcDirectory.TagKeywords);
} catch (NullReferenceException e) {
    imageFileInfo.Keywords = "valor padrao"
}

Even better to use try catch than if to test if the error exists, so processing is reduced when there is no error.

Observing

You’re getting NullException on the line

if (IPTCDirectory.GetDescription(IptcDirectory.TagKeywords) == null)

probably because the object IptcDirectory does not own the property TagKeywords, the right way to test:

if (IptcDirectory.GetProperty('TagKeywords') == null) {
    imageFileInfo.Keywords = "valor padrao";
} else {
    imageFileInfo.Keywords = IPTCDirectory.GetDescription(IptcDirectory.TagKeywords);
}

Put that code with if is less efficient than try catch.

EDIT

As pointed out by Maniero, actually Exceptions in C# are very expensive, so the structure if else is not less efficient at first (it all depends on how expensive GetProperty is in relation to the making of exceptions)

  • 1

    Why is it less efficient?

  • In the case of try catch, when there is no error, only the instruction imageFileInfo.Keywords = IPTCDirectory.GetDescription(IptcDirectory.TagKeywords); is executed. Already in the case of if at least two instructions, that of the if and the allocation.

  • 1

    But when he gives the exception he is absurdly (orders of magnitude) less efficient and he says he expects him to have exceptions, so he is no longer efficient. That’s why I always say, do not use exception to control flow, use only for exceptional situations. Every time you catch NullReferenceException is trying to fix with code something that must be fixed in the code. It is programming error.

  • 1

    @bigown, I have more experience in python where we usually use philosophy: it’s easier to Ask forgiveness than it is to get permission, but you have reason to the case of C# the exceptions are very expensive. I will change the answer.

  • Now it has become clearer, so far I believe that your response is the most appropriate one. I don’t know the weight of a Try catch, but probably if it’s less heavy then I’ll use the if. Incidentally, his reply was very satisfying in the doubt of "because the if indicated that it was null being that I was just asking if it was null"

  • There is no Getproperty function in this library: https://github.com/drewnoakes/metadata-extractor

  • @Leonardobonetti , reviewing the documentation here, it seems that the correct syntax is: typeof(IptcDirectory).GetProperty("TagKeywords"). See if it works... GetProperty is a class method Type and not his lilbrary.

Show 2 more comments

0

In this case you should separate each of the instructions into blocks of try catch specific.

for (int i = 0; i < numberOfProcessAvailable; i++) {
    var EXIFDirectory = directories.OfType<ExifDirectoryBase().FirstOrDefault();

    try {
        imageFileInfo.LensModel = Convert.ToString(EXIFDirectory.GetDescription(ExifIfd0Directory.TagLensModel));
    } catch (NullReferenceException e){ //tratar excecao }

    try {    
        imageFileInfo.Make = Convert.ToString(EXIFDirectory.GetDescription(ExifIfd0Directory.TagMake));
    } catch (NullReferenceException e){ //tratar excecao }

    try {
        imageFileInfo.Model = Convert.ToString(EXIFDirectory.GetDescription(ExifIfd0Directory.TagModel));
    } catch (NullReferenceException e){ //tratar excecao }
}

That way all the blocks would always run.

PS.: You could treat this problem also by using the instruction goto, but this usually makes the code less readable and more complicated.

Ps2.: I had not made explicit because I thought it was obvious.. But exceptions should always be dealt with and be as specific as possible

  • 1

    Danilo, this is good practice?

  • As far as I know, yes. That way it becomes clear that each of the instructions may fail, but that the others must be executed. This code is quite efficient since it does not process anything else if no exception occurs.

  • Thanks Danilo, helped me a lot, I always avoid repeating code but in this case it will be necessary !

  • 1

    If the number of variables is not known or is too large you can put the block try catch within a loop that iterates over them.

  • 5

    This answer is terrible.

  • 3

    LINQ, a more constructive criticism would help me to improve.

Show 1 more comment

Browser other questions tagged

You are not signed in. Login or sign up in order to post.