2

I have several controllers. In each controller class I defined the same set of helper methods:

private void info(String msg)
{
    ApexPages.addMessage(new ApexPages.Message(ApexPages.Severity.INFO, msg));
}

private void error(String msg)
{
    ApexPages.addMessage(new ApexPages.Message(ApexPages.Severity.ERROR, msg));
}

Credit: ConnectorController.cls

I try to follow coding best practices, thus I need to avoid code duplication. Now I am thinking how to do it right way. I am considering two options:

  1. Create an abstract class and then inherit all controller classes from it. But I don't feel that's a correct approach since info/error methods bear utility nature. That said, I still wanted to know whether my thinking is correct.

  2. Create a utility class and make those methods static. Then call those methods from the controller.

This question may be considered as too broad or opinion-based, but as long as best practices are concerned I don't believe many different views will pop up.

  • 1
    You are correct. Your question is too broad, and it might depend on your project's prerequisites. I do want to point out that the second option is, in my opinion, the best approach for your situation (that's what I did in one of my projects, which had lots of Visualforce pages - just one observation: always use msg+'' because if msg is null, then you'll get an exception). – Renato Oliveira Jul 31 '18 at 14:09
  • 2
    Asking for "Best Practice" advice is really just that - advice. Asking how to refactor code which you've re-used in multiple places into a different structure is a lot more on topic than soliciting options. – battery.cord Jul 31 '18 at 14:11
2

Personally, I wouldn't make any effort to avoid duplicating those single lines. If you write code with an IDE, then auto-complete and/or templates would get you those lines pretty quickly.

But, if you do want to wrap them up:

  1. Yes, you're right, inheritance wouldn't be a great idea. Generally, it's best to favour composition over inheritance. There are lots of references for that, here's the first reasonable one I found on google: https://en.wikipedia.org/wiki/Composition_over_inheritance
  2. If you're going to make a utility class, then it's probably best not to go with static methods. Static methods can't be overridden, and don't support interfaces

If I were to go full-on with OO stuff (probably too much, see Enterprise Hello World), then I would do it like this:

interface ApexPagesMessenger {
    void error(String msg);
    void info(String msg);
}

class DefaultApexPagesMessenger implements ApexPagesMessenger {
    public void info(String msg) {
        ApexPages.addMessage(new ApexPages.Message(ApexPages.Severity.INFO, msg));
    }

    public void error(String msg) {
        ApexPages.addMessage(new ApexPages.Message(ApexPages.Severity.ERROR, msg));
    }
}


class ApexPagesMessengerFactory {
    public static ApexPagesMessenger newInstance() {
        return new DefaultApexPagesMessenger();
    }
}

class MyController {
    private ApexPagesMessenger messenger = ApexPagesMessengerFactory.newInstance();
}

What does this buy you? The possibility of having a family of different implementations of ApexPagesMessenger, which could potentially be swapped in/out in different ways.

Edit

Of course, you could take a middle-ground approach to this and skip the factory i.e.

class MyController {
    private ApexPagesMessenger messenger = new DefaultApexPagesMessenger();
}

And then refactor later if it makes sense to.

  • 5
    I don't think the interface adds any value here. – Adrian Larson Jul 31 '18 at 14:30
  • @AdrianLarson You wrote that while I was typing a simpler version. It is debatable and, IMO, worth mentioning both approaches in a best practice discussion – Aidan Jul 31 '18 at 14:31
  • @Aidan can you please explain why one would want to override or implement an interface for a utility method? My understanding is that utility method should be static and final since they are just utility. – Eduard Aug 4 '18 at 7:04
  • 1
    Sending a message to the user is an abstract concept, and sending them a Visualforce Page Message is an instance of that abstract concept. So, you could build an alternative messaging implementation e.g. I once built something like Page Messages for a Lightning Component. If you have that sort of plugin-in architecture, then interfaces are a good way to express it. Then you could have one controller for both VF and Lightning, configured with plugins to deal with the differences between the two. – Aidan Aug 4 '18 at 7:32
2

Static Utility class, hands down. These are nice, contained methods, which helps reduce some of the verbosity of the ApexPages.AddMessage(new ApexPages.Severity.FATAL, '') into Messages.AddError('').

If you already had a parent class for your page extensions/controllers, I can see an argument being made to add it there (especially since it opens up the door for unique customizations for messages without changing, say, the save method).

You don't however, and this code is very simple - it just needs a home to live in between your different classes. Even if later you decided to use inheritance to create a structure of controllers, you can use the static methods as part of your controller calls, say as the default behavior.

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.