Where should validation of Id be placed?

Question

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;
}

Answer 1

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.