How to avoid redundant try-catch statements when there might be null pointers?

Question

I m experimenting with "new" libraries i found online by taking a close look at a covid tracing app. I scan a QR code, decode base64 to get the shop data in a Json string. I then use Gson to parse into a Shop POJO.

public class Shop {
    public metadata metadata;
    public String nameZh;
    public String nameEn;
    public String type;
    public String hash;

    public class metadata{
        public String typeEn;
        public String typeZh;

        public String getTypeEn() { return this.typeEn; }
        public String getTypeZh() { return this.typeZh; }
    }

    public metadata getMetadata() { return this.metadata; }
    public String getNameZh() { return this.nameZh; }
    public String getNameEn() { return this.nameEn; }
    public String getType() { return this.type; }
    public String getHash() { return  this.hash; }
    public String toString() {
        return getMetadata().getTypeEn()+", "+getMetadata().getTypeZh()+", "
                +getNameZh()+", "+getNameEn()+", "+ getType()+", "+getHash();
    }
}

However, when i want to write into an excel file (.xlsx) using apache POI, I find that not all shops have all the fields, meaning that some are null. This is nullpointer exception i got.

Exception in thread "main" java.lang.NullPointerException: Cannot invoke "Shop$metadata.getTypeEn()" because the return value of "Shop.getMetadata()" is null
    at xlsxWorker.writeShop(xlsxWorker.java:77)
    at xlsxWorker.writeOutput(xlsxWorker.java:51)

While it is possible for me to use try catch to avoid this, the resulting code has many try catch.

private static void writeShop(Row current, Shop shop){
        Cell cell = current.createCell(0);
        try{
            cell.setCellValue(shop.getMetadata().getTypeEn());
        }catch (NullPointerException e){
            cell.setCellValue("null");
        }
        cell = current.createCell(1);
        try{
            cell.setCellValue(shop.getMetadata().getTypeZh());
        }catch (NullPointerException e){
            cell.setCellValue("null");
        }
        cell = current.createCell(2);
        try{
            cell.setCellValue(shop.getNameZh());
        }catch (NullPointerException e){
            cell.setCellValue("null");
        }
        cell = current.createCell(3);
        try{
            cell.setCellValue(shop.getNameEn());
        }catch (NullPointerException e){
            cell.setCellValue("null");
        }
        cell = current.createCell(4);
        try{
            cell.setCellValue(shop.getType());
        }catch (NullPointerException e){
            cell.setCellValue("null");
        }
        cell = current.createCell(5);
        try{
            cell.setCellValue(shop.getHash());
        }catch (NullPointerException e){
            cell.setCellValue("null");
        }
    }

How do I avoid the excessive use of try-catches here? How can I simplify the redundancies

Дмитрий Морозов · Accepted Answer · 2021-06-24T16:42:26.300

2

You can initialize your Pojo with default values like

Empty String ""
null as a String literal "null"

Update

You can create new Metadata with default values too or use this for example

if(shop.getMetadata() != null)
{ 
//your brilliant code 
}

Update

'MapStruct' can help you with Mapping

edited Jun 24 '21 at 16:42

answered Jun 24 '21 at 16:24

Дмитрий Морозов

58
6

Hi, Дмитрий Морозов, While this could help eliminate for the normal String field in the Shop POJO, it didnt really help for the metadata inner class, because when it is null, i cant access the inner methods regardless. – experiment unit 1998X Jun 24 '21 at 16:33
You can create new Metadata with default values. or if(shop.getMetadata() != null){ your code } – Дмитрий Морозов Jun 24 '21 at 16:36
oh, that sounds like a good idea! i ll try it right now – experiment unit 1998X Jun 24 '21 at 16:38
it did not work. I am still getting the null pointer. I believe it is because the Shop is not being instantiated normally, but the values are being reflected because of the Gson, so it does not see the "default" values in shop – experiment unit 1998X Jun 24 '21 at 16:41
I tried the if statement for getMetadata() but just lead to more unnecessary code, because after checking metadata is not null, i have to ensure typeEn and typeZh is also not null – experiment unit 1998X Jun 24 '21 at 16:43
Are you sure that your both Shop and Metadata have all field initialized by something different of null? – Дмитрий Морозов Jun 24 '21 at 16:50
yes, i put metadata metadata = new metadata() – experiment unit 1998X Jun 24 '21 at 16:53
Metadata also must have initialized fields =) – Дмитрий Морозов Jun 24 '21 at 16:53
yes, metadata has initialized fields to "", and also has constructor public metadata() {}, but it still has error, maybe if i set the value inside the constructor like this.typeEn ="" it might be different – experiment unit 1998X Jun 24 '21 at 16:57
maybe the null came from JSON. Can you configure your JSON parser to ignore null values(Don't set null values)? Also, you can improve setters like setA(String a){if (a != null) this.a = a} I am pretty sure that avoid null values is much better than use -try catch- – Дмитрий Морозов Jun 24 '21 at 17:03
ah yes, the null came from the JSON, it has "metadata":null, I m not using any setters, so if i want to avoid the null values, i might have to make custom Json deserializer to avoid the null values? – experiment unit 1998X Jun 24 '21 at 17:12
Can you configure your parser to use setters? Maybe it's easier to use Jackson? It has wide and flexible configuration options – Дмитрий Морозов Jun 24 '21 at 17:15
try modifying getters like getMetadata(){if this.metadata == null} this.metadata = new Metadata; return this.metadata; After it will be impossible to get null – Дмитрий Морозов Jun 24 '21 at 17:17
The Gson avoids serializing nulls, but I found that I can use serializeNull() and then use ternary operators as such to address the issue when not using POJO. https://stackoverflow.com/questions/33435683/deserialize-with-gson-and-null-values/33435722 – experiment unit 1998X Jun 24 '21 at 17:19
Thank you for your help Дмитрий Морозов. While your comments and answers contain helpful common sense work-arounds, I found that the other solution is much simpler. As such, I am unable to accept your answer – experiment unit 1998X Jun 24 '21 at 17:24
This IS the right answer. Upvoted: `if(shop.getMetadata() != null)` is what the OP needs. – terrorrussia-keeps-killing Jun 26 '21 at 07:40

Richard Rowell · Answer 2 · 2021-07-03T05:30:03.963

1

I'd use a helper method:

private static void setCellValue_printNullOnNPE(Cell cell, Supplier<String> value){
    String text = "null";
    try{ 
        text = value.get()
    }catch(NullPointerException e){}
    cell.setCellValue( text );
}

Then your writeShop method will look like this:

Cell cell = current.createCell(0);
setCellValue(cell, () -> shop.getMetadata().getTypeEn());
cell = current.createCell(1);
setCellValue(cell, () -> shop.getMetadata().getTypeZh());

edited Jul 03 '21 at 05:30

answered Jun 24 '21 at 16:44

Richard Rowell

318
1
8

This works, but for some reason, my version of java requires a return statement setCellValue(cell, ()->{ return shop.getMetadata().getTypeEn(); }); – experiment unit 1998X Jun 24 '21 at 16:51
1

I can't tell how bad this solution is. This is: 1) abusing `NullPointerException` including the fact it's caught (bonus: what if the supplier is null? bonus 2: what if the supplier implementation is wrong therefore throwing an NPE?); 2) silently suppressing the caught exception is just bad; 3) this is probably where `java.util.Optional` may play well (despite I don't like that idea too); 4) `shop.metadata != null ? shop.metadata.typeEn : null` is sufficient for these two particular cases. Downvoted. – terrorrussia-keeps-killing Jun 26 '21 at 07:36
Thank you for your input. I have taken into consideration your comments and am using ternary operators cell.setCellValue(shop.getMetadata() != null ? shop.getMetadata().getTypeEn() : "null"); although i would certainly love concise and simple code, i guess i didnt consider anything "deeper" regarding the solution. – experiment unit 1998X Jun 26 '21 at 08:53
@experimentunit1998X Making it concise not necessarily makes it better from many perspectives and it is also definitely not the only aim when writing code. Option #4 is the fastest solution, it does not suffer for performance overheads (like `java.util.Optional` despite folks love using it), but may suffer from typos. C#, Kotlin have something to reduce the boilerplate https://stackoverflow.com/questions/48253107/what-does-do-in-kotlin-elvis-operator making it also more robust. If Java would support metaprogramming at compile time, you'd construct it as a macro. But it is Java after all. – terrorrussia-keeps-killing Jun 26 '21 at 09:58
1

@experimentunit1998X Here is what that can be considered concise: `static void writeShop(final Row row, final Shop shop) { writeCell(row, 0, shop.metadata != null ? shop.metadata.typeEn : null); writeCell(row, 1, shop.metadata != null ? shop.metadata.typeZh : null); writeCell(row, 2, shop.nameZh); writeCell(row, 3, shop.nameEn); writeCell(row, 4, shop.type); writeCell(row, 5, shop.hash); } private static void writeCell(final Row row, final int i, @Nullable final String value) { row.createCell(i).setCellValue(value); }` Note that it does not use `"null"` that only pretends to be a `null`. – terrorrussia-keeps-killing Jun 26 '21 at 10:01
@experimentunit1998X _@Дмитрий-Морозов_'s answer might be even more simple: extract the metadata null-checks and put writing cells 0 and 1 into the `if` statement. – terrorrussia-keeps-killing Jun 26 '21 at 10:02
i see! so the get method wouldnt throw error because some of the get methods return a null, but the error was thrown because i tried to call a get method on the null value. I can directly set the null value into the cell, and I wouldnt need to exactly check if the metadata field variables are null so long as i know metadata itself is not null. – experiment unit 1998X Jun 26 '21 at 10:30
in that case, all the try catches were unnecessary from the beginning! because the get would just return a null value that when set into the cell would not throw any errors, just that the metadata null was throwing error... and this question was in a sense unnecessary! The if statement would certainly be more concise considering i only needed to ensure metadata is not null so that i can call metadata methods... – experiment unit 1998X Jun 26 '21 at 10:36

How to avoid redundant try-catch statements when there might be null pointers?

2 Answers2