function readOnly(count){ }
Starting November 20, the site will be set to read-only. On December 4, 2023,
forum discussions will move to the Trailblazer Community.
+ Start a Discussion
ethan huntethan hunt 

Returning null in catch block - apex

Is returning null in catch block is best practice..

String strOpportunityRecord;
        Opportunity sourceOpportunityRecord = new Opportunity();
        try {
            strOpportunityRecord =  'SELECT ' + Queriablefields +'Name From Opportunity where id =: sourceOpportunityId';
            if (strOpportunityRecord != null) {
                sourceOpportunityRecord = Database.query(strOpportunityRecord) ;
            }
           return sourceOpportunityRecord;
        }
        catch(QueryException ex) {
            system.debug(ex);
            return null;
        }
    }
Benajmin PirihBenajmin Pirih

Best is a relative word.

If you want to swallow / disregard the exception then you would return null.  i.e. you cannot do anything about the exception but still want to continue processing then this might be "best". 

"best" often means doing something with an exception.  Be it using .addError( ) to notify the user, throwing a custom exception, or performing some other error handling corrective logic. 

So I would say this is not best practice but maybe the author was expecting to fail and didn't want to stop execution. Personally I like to know when my applications fail and swallowing the exception prevents error reporting so I would venture to say no.. not best practice but might be intentional.

ethan huntethan hunt
Hi Benajmin,

Thanks for your help. If u dont mind I have another query .. Could you please help me resoving the issue.  

if I am having a class
---------------------------------------------------------
public class TVRemoteControl {
    // Volume to be modified
    Integer volume;
    // Constant for maximum volume value
    static final Integer MAX_VOLUME = 50;  
  
    // Constructor
    public TVRemoteControl(Integer v) {
        // Set initial value for volume
        volume = v;
    }
      
    public Integer increaseVolume(Integer amount) {
        volume += amount;
        if (volume > MAX_VOLUME) {
            volume = MAX_VOLUME;
        }
        return volume;
    }
  
    public Integer decreaseVolume(Integer amount) {
        volume -= amount;
        if (volume < 0) {
            volume = 0;
        }
        return volume;
    }  
  
    public static String getMenuOptions() {
        return 'AUDIO SETTINGS - VIDEO SETTINGS';
    }
     
}
--------------------------------------------------------------
When I will write the test case -

@isTest
class TVRemoteControlTest {
    @isTest static void testVolumeIncrease() {
        TVRemoteControl rc = new TVRemoteControl(10);
        Integer newVolume = rc.increaseVolume(15);
        System.assertEquals(25, newVolume);
    }
  
    @isTest static void testVolumeDecrease() {
        TVRemoteControl rc = new TVRemoteControl(20);
        Integer newVolume = rc.decreaseVolume(15);
        System.assertEquals(5, newVolume);      
    }
      
    @isTest static void testVolumeIncreaseOverMax() {
        TVRemoteControl rc = new TVRemoteControl(10);
        Integer newVolume = rc.increaseVolume(100);
        System.assertEquals(50, newVolume);      
    }
  
    @isTest static void testVolumeDecreaseUnderMin() {
        TVRemoteControl rc = new TVRemoteControl(10);
        Integer newVolume = rc.decreaseVolume(100);
        System.assertEquals(0, newVolume);      
    }
  
    @isTest static void testGetMenuOptions() {
        // Static method call. No need to create a class instance.
        String menu = TVRemoteControl.getMenuOptions();
        System.assertNotEquals(null, menu);
        System.assertNotEquals('', menu);
    }
}
--------------------------------------------
Can TVRemoteControl rc = new TVRemoteControl be initialised once and can be reused.

Regards
crop1645v2crop1645v2
wrto best practice on returning null in a catch block - your example is a specific use case wherein a QueryException (no record found) is acceptable to your logic; returning null on a DmlException would not be a good idea until you have done addError() or rollbacks.

Your code would look cleaner if it were converted into a method and looked like this:

<pre>
Opportunity getOppo(ID sourceOpportunityId, String queriableFields) {
    // queriableFields guaranteed to be in form 'fldA,fldB,...,'
    Opportunity res;
    try {
        res = Database.query('SELECT ' + queriableFields +'Name From Opportunity where id =: sourceOpportunityId');
    }
    catch (QueryException e) {}
    return res;
}
</pre>
crop1645v2crop1645v2
wrto to your test class - yes - you have way too many testmethods

<pre>
@isTest
class TVRemoteControlTest {
    @isTest static void testRemoteControlActions() {
        TVRemoteControl rc = new TVRemoteControl(10);
        // 0.1 test increaseVolume
        Integer newVolume = rc.increaseVolume(15);
        System.assertEquals(25, newVolume);  // 10 + 15 = 25
        // 0.2 test decreaseVolume
        newVolume = rc.decreaseVolume(15);
        System.assertEquals(10, newVolume);  // 25 - 15 = 10
        // and so on
    }
}
</pre>