2

BACKGROUND

I have the following visualforce page (this is just for the completeness of the example) and custom controller:

Visualforce page:

<apex:page id="documentStatusPage" controller="DocumentStatusController"
            sidebar="false" showHeader="false"
            doctype="html-5.0" cache="false">

    <header>
        <h1>Doc name: {! document.Name }</h1>
    </header>

</apex:page>

Controller:

public with sharing class DocumentStatusController
{
    private transient DocumentWrapper document;

    public DocumentStatusController(){}

    private Id getDocumentId()
    {
        return (Id) ApexPages.currentPage().getParameters().get('documentId');
    }

    public DocumentWrapper getDocument()
    {
        if (this.document == null)
        {
          Document__c doc = [SELECT Id, Name FROM Document__c WHERE Id =: getDocumentId()];

          DocumentWrapper documentWrapper = new DocumentWrapper();
          documentWrapper.Name = doc.Name;
          this.document = documentWrapper;
        }

        return this.document;
    }

    /**************************************
    * Inner Classes
    ***************************************/
    public class DocumentWrapper
    {
        public String Name { get; set; }
    }

}

PROBLEM

When Page Parameters contain the 'documentId' parameter there are basically two situations: id is valid (null or 'a050S000000xxxx'), or id is not valid ('invalid_value_id').

When this line of code gets executed return (Id) ApexPages.currentPage().getParameters().get('documentId'); an exception will be raised:

System.StringException: Invalid id: invalid_value_id

--Update-- I also want to be able communicate back to user what the exact error was: whether the Id was invalid or the 'documentId' parameter was missing. I assume this requires a mechanism that distinguishes between these two cases. --Update--

QUESTION

I am wondering what is the best way to handle this problem?

One way I see is utilize a try/catch block. But the exact question here where should I put this block:

(1) in the getDocumentId() method itself and return null if exception is thrown?

 private Id getDocumentId()
 {
    try
    {
      return (Id) ApexPages.currentPage().getParameters().get('documentId');
    }
    catch (Exception ex)
    {
      return null;
    }
 }

OR

(2) in the caller of getDocumentId()?

public DocumentWrapper getDocument()
{
    if (this.document == null)
    {
      try
      {
        Document__c doc = [SELECT Id, Name FROM Document__c WHERE Id =: getDocumentId()];
      }
      catch (Exception ex)
      {
        this.document = null;
        ApexPages.addMessage(new ApexPages.Message(ApexPages.Severity.ERROR, ex.getMessage()));
      }
    }

    return this.document;
}
2

I will prefer to consolidate into a single method as follows as because you are using private getDocumentId() only for fetching document. So, better to use in one method itself.

public DocumentWrapper getDocument()
{
    if (this.document == null)
    {
      try
      {
        Id docId = (Id) ApexPages.currentPage().getParameters().get('documentId');
        Document__c doc = [SELECT Id, Name FROM Document__c WHERE Id =:docId];
      }
      catch (Exception ex)
      {
        this.document = null;
        ApexPages.addMessage(new ApexPages.Message(ApexPages.Severity.ERROR, ex.getMessage()));
      }
    }

    return this.document;
}

Update

Based on your updated question refer this approach of error handling.

private Id getDocumentId()
 {
    try
    {
        String strId = ApexPages.currentPage().getParameters().get('documentId');
        if(String.isEmpty(strId))
        {
            throw new CustomException('Document Id is Null');
        }
        return (Id) strId;
    }
    catch (Exception ex)
    {
      throw new CustomException('Id is invalid');
      return null;
    }
 }

The getDocument() will remain same what you have written.

  • thanks for your answer. I see this as a good option if you need access the 'documentId' param once. But if several methods in my controller make use of this parameter, wouldn't it be better to encapsulate it into a private getter method? – Eduard Jul 25 '18 at 13:54
  • Refer my updated answer which handles all scenarios – Santanu Boral Jul 25 '18 at 14:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.