3

I am beginner in Salesforce and I have to verify if the field Order.ShippingCountry is filled using the OrderId.

I would like to reuse this method below to fetch the Order.ShippingCountry. But I was wondering if it is a good practice to use Order as a return. Ideally, do I have to create another method to return a List<Order> ?

public class OrderSelector {

    public static Order getOrderAndOrderItemsById(Id orderId){

            return [SELECT Id,OwnerId,EffectiveDate,ShippingCountry,
                    AccountId,Account.Name
                    (SELECT Id, Quantity, UnitPrice, 
                     Product2.ProductCode,Product2.Name,Product2.Family
                     FROM OrderItems)
                    FROM Order
                    WHERE Id = :orderId];
        }}

Using the current version of the method above, my method will be like it:

public class ValidateOrder {

    public class ValidateOrderException extends Exception{}

    private static void validateShippingCountryByOrderId(Id orderId) {

          Order varOrder = OrderSelector.getOrderAndOrderItemsById(orderId);

          if(varOrder.ShippingCountry.isEmpty()){
            throw new ValidateOrderException(Label.InvalidateDeleteDocument);
          }
    }

}

Is my code following best practices? Or would it be better to apply List<Order> varOder ? Also, as a good practice, would it be better to create another method in OrderSelector class to fetch only the Order.ShippingCountry and not all these fields?

2 Answers 2

5

In my experience, selector methods should, in general:

  • accept collections as input and return collections as output. This avoids null checks.

  • not be static as it facilitates mocking patterns such as ApexMocks which as you evolve your experience, you may come to favor

    public class OrderSelector {
    
      public Order[] getOrderAndOrderItemsById(Set<Id> orderIds){
          orderIds.isEmpty()
           ? new List<Order>()
           : return [SELECT Id,OwnerId,EffectiveDate,ShippingCountry,
                  AccountId,Account.Name
                  (SELECT Id, Quantity, UnitPrice, 
                   Product2.ProductCode,Product2.Name,Product2.Family
                   FROM OrderItems)
                  FROM Order
                  WHERE Id IN :orderIds];
      }}
    

If you follow the Lightning Enterprise Architecture patterns, the method name would be changed to

selectByOrderIds(Set<Id> orderIds) or
selectById(Set<Id> orderIds) // less is more  

as the method is part of an OrdersSelector, so by definition, you don't need to repeat Order in the method name.

You could also name it:

selectWithOrderItemsById(Set<Id> orderIds)

if you want to be clear that the return includes OrderItems

2
  • You said this have to return collections as output, but Is Order[] a collection? And could Order[] recieve more than one record?
    – Cline
    Apr 6, 2022 at 19:02
  • 1
    Order[] is same as List<Order>; fewer characters to type :-)
    – cropredy
    Apr 6, 2022 at 19:57
2

It depends. As written, if the user doesn't have access to the record, or the record Id is invalid, you'll get a "No rows for assignment". You might want more explicit error handling in this event.

public static Order getOrderAndOrderItemsById(Id orderId){
  try {
    return [SELECT Id,OwnerId,EffectiveDate,ShippingCountry,AccountId,Account.Name
             (SELECT Id, Quantity, UnitPrice, Product2.ProductCode,Product2.Name,Product2.Family
                 FROM OrderItems)
                FROM Order
                WHERE Id = :orderId];
    } catch(ListException e) {
      throw new CustomExceptions.InvalidIdOrInsufficientAccess(recordId);
    }
}

That aside, I would generally prefer accepting a list of Id values, and returning a list of Order records, because you'll need it more likely than not, and you'll avoid accidentally writing SOQL inside a for loop later down the line:

for(Order record: Trigger.new) {
  Order queriedRecord = OrderSelector.getOrderAndOrderItemsById(record.Id);
  // etc...
}

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.