1

I'm trying to understand when to use private set in singleton vs not using private set. For example:

public static Boolean runningInASandbox {
get {
  if (runningInASandbox == null) {
    runningInASandbox = [SELECT IsSandbox FROM Organization LIMIT 1]
    .IsSandbox;
  }
  return runningInASandbox;
}
  private set;
}

I've combed the internet but was unable to find an explanation that made sense to me. When should private set be used here vs not? I'm guessing in most cases, private set can be used.

3 Answers 3

3

What you have here is not a singleton pattern. The singleton pattern is one that restricts the instantiation of a class to just a single instance (see, for example, Geeks for geeks, Refactoring Guru, and Wikipedia). You haven't restricted Boolean from being used anywhere else in the system (nor could you), so this isn't a singleton pattern. Most developers would simply call this a publicly read-only static variable.

With that out of the way, the other answers point out that unit tests will be crippled with this design, and I agree. Unlike the other answers, I would suggest that the better solution would be to have a private test-visible backing variable. This allows your code to still be safe from accidental overwrites by a developer just trying to get something to work in production code, rather than only for unit tests.

As such, my approach to this would look something more like this:

@TestVisible private static Boolean private_IsSandboxFlag;
public static Boolean isSandbox {
  public get {
    if(private_IsSandboxFlag == null) {
      private_IsSandboxFlag = [SELECT IsSandbox FROM Organization].IsSandbox;
    }
    return private_IsSandboxFlag;
  }
  private set;
}

This prohibits live code from altering the flag at compile-time, but allows unit tests to set a value in order to test alternative branches of code that depend on this flag. As an experienced developer, I've seen this happen far too many times by accident. That's because the assignment operator = returns the r-value after assigning the r-value to the l-value. In other words, without the private set, it's possible to accidentally write:

if(Utils.isSandbox = true) {

Or:

while(Utils.isSandbox = true && hasMoreData()) {

Etc.

The private set is there for safety. We don't want to write to the variable at all in production code, so we lock it down to prevent accidents that can be hard to track down when you've got a lot of code to look through.

2
  • Yeah +1 for @TestVisible and correction of terminology. Although normally when using a static flag like in the OP I don't even need the public getter as I only consume internally.
    – Adrian Larson
    Mar 12 at 13:39
  • Wow, thank you so much for detailed answer!
    – snippet69
    Mar 12 at 22:33
1

In your example, I'd highly recommend using a public set. Why?

So you can write test methods that inject a boolean to the property runningInASandbox.

You are going to have (or should have) test methods that will need to run in both sandboxes and PROD. Since a test method should always be coded using test isolation principles, this means that the same test should have the same behavior whether it runs in PROD or sandbox.

Thus, if you have certain method behavior that should happen when running in PROD, have the test method include a statement

MyClass.isRunningInASandbox = false;
...
asserts for PROD expectations

Similarly, if you expect different behavior when running in a sandbox, the test method (a different one) would be:

MyClass.isRunningInASandbox = true;
...
asserts for Sandbox expectations

This principle can be extended to other useful properties to adhere to the test isolation principle, notably

public static Datetime now {
   get {
     return now == null ? Datetime.now() : now;
   }
   set;
}

public static Date today {
   get {
     return now == null ? Date.today() : today;
   }
   set;
}

where all references in your code to the current time or date are never coded as System.now() or Date.today() but instead are coded as MyClass.now and MyClass.today. Then your test methods can inject fixed datetimes or dates where it is important to always run the code-under-test with an invariant time/date.

3
  • 1
    A small critique: use "test method" or "IsTest method" instead of "testmethod" in the explanation to avoid mention of the deprecated "testmethod" keyword...
    – Phil W
    Mar 12 at 9:58
  • While you're not wrong in the usual sense, I would avoid public set because it allows developers to accidentally write if(MyClass.isRunningAsSandbox = true), which would behave differently than if(MyClass.isRunningAsSandbox == true).
    – sfdcfox
    Mar 12 at 13:25
  • good points; i cleaned up testmethod to test method; as for sfdcfox point; I guess I don't make this mistake because the property name suggests coding if(MyClass.isRunningInASandbox) {...} and I guess I design for my own style/convenience.
    – cropredy
    Mar 12 at 17:56
0

When you use private access modifier on the Apex property, the property cannot be modified by other Apex classes.

So in your scenario, since you have private setter that means only the class (your current class) that has the runningInASandbox method can set it and other class cannot set this value. They can only get the value. Much useful if you really want to lock the variable modification to strictly to the current Apex class.

While making it public would mean, any class can modify it. Useful if you want the variable to be modified by other classes using it.

1
  • 1
    Allowing public would also allow if(SomeClass.runningInASandbox = true), which would have unexpected side effects in code.
    – sfdcfox
    Mar 12 at 13:35

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .