Proper placement of Try/Catch blocks

It was recommended by a member to begin putting try/catch blocks in my trigger in order to catch Dml and null pointer exceptions. The issue is when I place them it makes my entire program complain. Right now I’m just putting them where I think they would be helpful, but I really don’t have a good grasp on it. I have a feeling I’m going to get a few down votes for this but my random placement of the blocks isn’t effective..

My test class

@IsTest
public class OverrideTest {

static testmethod void addressOverride() {

    Sampling__c s = new Sampling__c();

    Validation_Region__c region = new Validation_Region__c();
    region.Country_Key__c = 'HK';
    region.Description__c = 'Luxembourg';

    insert region;

    s.Quantity_of_Samples__c = 2;
    s.Override__c = 'Yes';
    s.Country__c = 'hk';
    s.State_Province__c = 'Luxembourg';
    //s.Zip_Postal_Code__c = ''; 
    //s.Zip_Postal_Code__c = ''; 
    //s.Zip_Postal_Code__c = null; 


    try {
        insert S;
    } catch (DmlException e) {
        system.assert(false, e.getMessage() + ' ' + e.getLineNumber());
    }
}
}

So for instance if I was going to add a try/catch on to my for loop like so, it complains about my brackets, which in my experience means I’m missing something simple.

try{
for (Sampling__c s: Trigger.new) {
    if ((s.Country__c != null) && (S.Override__c == 'Yes')) {
        String countryKey = s.Country__c;
        String regionKey = s.State_Province__c;
        //countryKey = countryKey.toUpperCase();
        // regionKey = regionKey.toUpperCase(); This needs to get fixed possibly toUpperCase or change it to .equalsIgnoreCase

        if (validRegions.get(countryKey) != null && validRegions.get(countryKey).get(regionKey) != null) {
            // The Region belongs to the country
            Validation_Region__c vr = validRegions.get(countryKey).get(regionKey);
        } else {
            System.debug('Breaking at Country/Region Validation.');
            break;
        }                   
        }
}catch(Exeception e){//Complains
 System.debug('Stuff broke. '+e.getMessage());//Complains here
}

trigger OverrideTrigger on Sampling__c(before insert, before update) {

Set < String > requiresZipCode = new Set < String > {
    'AR', 'AT', 'BE', 'BG', 'BR', 'CA', 'CN', 'CZ', 'DE', 'DK', 'EG', 'ES', 'FI', 'FR', 'GB', 'GR', 'HR', 'HU', 'ID', 'IL', 'IN', 'IR', 'IT', 'JP', 'KR', 'KY', 'KZ', 'MT', 'MU', 'MY', 'NL', 'NO', 'PE', 'PH', 'PL', 'PT', 'RO', 'SE', 'SG', 'SI', 'SK', 'TN', 'TW', 'UA', 'US', 'ZA'
};
Set < String > optionalZipCode = new Set < String > {
    'AU', 'CH', 'CL', 'CO', 'HK', 'IE', 'MX', 'NZ', 'RU', 'TH', 'TR', 'VE'
};

// Top level map is keyed by Country. Inner Map is keyed by Region   

Map < String, Map < String, Validation_Region__c >> validRegions = new Map < String, Map < String, Validation_Region__c >> ();

try{
for (Validation_Region__c objR: [Select Id, Country_Key__c, Description__c, Name FROM Validation_Region__c]) {       
    String countryKey = objR.Country_Key__c;      
    Map < String, Validation_Region__c > regionMap = validRegions.get(countryKey);
    // Maybe rework to use Map.containsKey rather than null check. Would be cleaner.
    if (regionMap == null) {
        regionMap = new Map < String, Validation_Region__c > ();
        validRegions.put(countryKey, regionMap);
    }
    string regionKey = objR.Description__c;
    regionMap.put(regionKey, objR);
}catch(DmlException e){
  System.debug('There has been a DML Exception ' +e.getMessage());
 }

for (Sampling__c s: Trigger.new) {
    if ((s.Country__c != null) && (S.Override__c == 'Yes')) {
        String countryKey = s.Country__c;
        String regionKey = s.State_Province__c;
        //countryKey = countryKey.toUpperCase();
        // regionKey = regionKey.toUpperCase(); This needs to get fixed possibly toUpperCase or change it to .equalsIgnoreCase

        if (validRegions.get(countryKey) != null && validRegions.get(countryKey).get(regionKey) != null) {
            // The Region belongs to the country
            Validation_Region__c vr = validRegions.get(countryKey).get(regionKey);
        } else {
            System.debug('Breaking at Country/Region Validation.');
            break;
        }                
        if ((s.Zip_Postal_Code__c != null) && (requiresZipCode.contains(countryKey))) {
            System.debug('Inside true Requires Zip code condition.');                   
            if ((countryKey == 'HK' || countryKey == 'IE') && (s.Zip_Postal_Code__c.length() > 0)) { //Condition for length of  0!
                System.debug('Inside ' + countryKey + ' this country does not have postal codes.');
                break;
            }
            if ((countryKey == 'TW') && (s.Zip_Postal_Code__c.length() <= 3)) { //Condition for length of  3!
                System.debug('Inside ' + countryKey + ' and its less than 3');
            }
            if ((countryKey == 'AR' || countryKey == 'AT' || countryKey == 'BE' || countryKey == 'CH' || countryKey == 'DK' || countryKey == 'HU' || countryKey == 'NO' || countryKey == 'PH' || countryKey == 'PI' || countryKey == 'SI' || countryKey == 'TN' || countryKey == 'VE' || countryKey == 'ZA' || countryKey == 'CH' || countryKey == 'NZ' || countryKey == 'TN' || countryKey == 'VE') && (s.Zip_Postal_Code__c.length() <= 4)) { //Condition for length of 4!
                System.debug('Inside ' + countryKey + ' that cant be larger than 4.');
            }
            if ((countryKey == 'DE' || countryKey == 'ES' || countryKey == 'FI' || countryKey == 'FR' || countryKey == 'GR' || countryKey == 'HR' || countryKey == 'ID' || countryKey == 'IL' || countryKey == 'IR' || countryKey == 'IT' || countryKey == 'MX' || countryKey == 'MY' || countryKey == 'US' || countryKey == 'MX' || countryKey == 'TH' || countryKey == 'TR') && (s.Zip_Postal_Code__c.length() <= 5)) { //Condition for length of 5!
                System.debug('Inside ' + countryKey + ' and its <= 5!');
            }
            if ((countryKey == 'CA' || countryKey == 'CN' || countryKey == 'CZ' || countryKey == 'IN' || countryKey == 'KZ' || countryKey == 'PL' || countryKey == 'SE' || countryKey == 'SG' || countryKey == 'SK' || countryKey == 'RU' || countryKey == 'CO') && (s.Zip_Postal_Code__c.length() <= 6)) { //Condition for length of 6!
                System.debug('Inside ' + countryKey + ' And its less than 6!');
            }
            if ((countryKey == 'JP' || countryKey == 'KR' || countryKey == 'NL' || countryKey == 'RO' || countryKey == 'AU' || countryKey == 'CL') && (s.Zip_Postal_Code__c.length() <= 7)) { //Condition for length of 7!
                System.debug('Inside ' + countryKey + ' And its less than 7!');
            }
            if ((countryKey == 'GB' || countryKey == 'BR') && (s.Zip_Postal_Code__c.length() <= 9)) { //Condition for length of 9!
                System.debug('Inside ' + countryKey + ' And its less than 9!');
            }
            if ((countryKey == 'US') && (s.Zip_Postal_Code__c.length() <= 10)) { //Condition for length of 9!
                System.debug('Inside ' + countryKey + ' And its less than 10!');
            }
            } 
            else if ((optionalZipCode.contains(countryKey) || (s.Zip_Postal_Code__c == null))) { 
            //////////WORK ON MORE VALIDATION RULES FOR REQUIRED AND OPTIONAL ZipCODES!!! Tomorrow 3/5/2014                                                                      
                    System.debug('Zipcode is null');
            if(!String.isBlank(s.Zip_Postal_Code__c)){   
            if ((countryKey == 'CH' || countryKey == 'NZ' || countryKey == 'TN' || countryKey == 'VE') && (s.Zip_Postal_Code__c.length() == 4)) { //Condition for length of  4!
                System.debug('Inside ' + countryKey + ' this zip code matches');
            }
            if ((countryKey == 'MX' || countryKey == 'TH' || countryKey == 'TR') && (s.Zip_Postal_Code__c.length() == 5)) { //Condition for length of  5!
                System.debug('Inside ' + countryKey + ' this zip code matches');
            }
            if ((countryKey == 'RU' || countryKey == 'CO') && (s.Zip_Postal_Code__c.length() == 6)) { //Condition for length of  6!
                System.debug('Inside ' + countryKey + ' this zip code matches');
            }
            if ((countryKey == 'AU' || countryKey == 'CL') && (s.Zip_Postal_Code__c.length() <= 7)) { //Condition for length of  7!
                System.debug('Inside ' + countryKey + ' this zip code matches');               
          } else {
            System.debug('The countryCode ' + countryKey + 's' + ' zipcode is not the proper length. There is an error inside the Zipcode Field');     
            break;       
          } 
        }
      } 
    } 
  }
}

Answers:

Thank you for visiting the Q&A section on Magenaut. Please note that all the answers may not help you solve the issue immediately. So please treat them as advisements. If you found the post helpful (or not), leave a comment & I’ll get back to you as soon as possible.

Method 1

Sometimes, it makes sense to try/catch, but not always. Some code will catch the exception: it might not be your code. You need to ask yourself a few things.

Can my code recover from the exception? If you can catch the exception and do something useful, then by all means, catch the exception and recover from the error.

Does the exception really matter? Sometimes, especially when invoking library/framework code, you might need to catch and eat (ignore) an exception that does not affect the overall processing of your program.

Can the exception be thrown? Does it make sense to catch a divide by zero exception if you already checked the divisor against zero and avoided dividing? Does it make sense to do something with a null pointer exception if you already know the variable cannot be null?

Can my code do something useful with the exception? If the code in question is low-level it might not make sense to do anything with the exception. Let it bubble higher up the call stack to another process that can actually do something useful with it.

There is no one-size-fits-all approach, but surrounding an entire method with try/catch is normally a code smell. It sounds like the advice you received was out of laziness: catch all exceptions so it does not fail. But those failures can be important: a null pointer or DML exception is valuable information that can direct you toward a logic error in your code so you can fix the bug. Eating that exception just makes your code more difficult to prove correct.

Method 2

Use try / catch in unit tests to test for expected DML outcomes

In your case:

try {
    insert S;
} catch (DmlException e) {
    system.assert(false, e.getMessage() + ' ' + e.getLineNumber());
}

Is making the assumption that the test data will fail the insert. And you probably want something more like:

try {
    insert S;
} catch (DmlException e) {
    system.assertEquals(true, e.getMessage().contains('my expected error');
}

The reason to use try / catch here is that the unit test can compile and run without triggering a system error but will still properly assert the outcome of the error (and that it is the right error). Using it your normal class will, as you’ve got it, allow you to control the debugging.

Specifically in your case, the bracket error is not due technically to the use of try/catch, you’re just missing a bracket towards the end:

}//add bracket here
} catch(Exeception e) {//Complains
 System.debug('Stuff broke. '+e.getMessage());//Complains here
}

Method 3

It was recommended by a member to begin putting try/catch blocks in my trigger in order to catch Dml and null pointer exceptions.

  • Try-catch within before triggers won’t catch any DML errors on the implicit DML done on Trigger.new as the DML occurs after the trigger executes and SFDC will take care of rolling back the transaction and surfacing an error
  • Try-catch or the equivalent Database.dmloperation within after triggers if such after triggers are doing DML is useful because there may be things you want to do – support partial updates, do full or partial rollbacks, log the error, and/or issue a more useful error message that has more context helping your user or sysad debug the problem
  • Try-catch within VF controller action methods, especially if they do DML, is useful for the same reasons as above
  • Sometimes, I’ll use try-catch blocks when invoking services not under my control – to ensure that if an error occurs, I do the proper action and surface a useful error message.


All methods was sourced from stackoverflow.com or stackexchange.com, is licensed under cc by-sa 2.5, cc by-sa 3.0 and cc by-sa 4.0

0 0 votes
Article Rating
Subscribe
Notify of
guest

0 Comments
Inline Feedbacks
View all comments
0
Would love your thoughts, please comment.x
()
x