[SalesForce] Changing access modifiers to accomodate unit tests

I've noticed that when writing classes, it will often be my first impulse to make certain properties private since they wouldn't need to be referenced by other classes. Take for example a class that generates random strings.

public class RandomStringGenerator{
    private final Integer StringLength = 8;
    private final list<String> ValidStringCharacters = new list<String>{'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z', '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a'};

    public String getRandomString(){
        String ReturnString = '';
        for (Integer i = 0; i < StringLength; i++){
            Double CharacterToAdd = Math.random() * (ValidStringCharacters.size() - 1);
            ReturnString += ValidStringCharacters[(Integer) CharacterToAdd];
        }
        return ReturnString;
    }
}

The two variables StringLength and ValidStringCharacters could be made private since all of the other classes could ignore the length of the string and the characters that it is composed of, and just work with what they get. That's fine for all classes that is, except for the test class, which would want to make assertions based on those values to verify that the class is working properly. 

@isTest
public class TestRandomStringGenerator{

    private static RandomStringGenerator rsg;

    static{
        rsg = new RandomStringGenerator();
    }

    public static @isTest void testStringLength(){
        system.assert(rsg.getRandomString().length() == rsg.StringLength);
    }

    public static @isTest void testStringCharacters(){
        Set<String> ValidCharacters = new Set<String>(rsg.ValidStringCharacters);
        String RandomString = rsg.getRandomString();
        for (Integer i = 0; i < RandomString.length(); i++){
            system.assert(ValidCharacters.contains(RandomString.substring(i,i+1)));
        }
    }

}

What is the preferred way to handle this situation? Should those private variables be made public just so that the test class can access them?

Best Answer

You should definitely not be changing implementation details for the sake of tests.

If you need private members to be available in your tests then you should use the @TestVisible annotation.

There is however the deeper question of, does your test class need to use these variables at all? The answer is (most probably) no.

Think about what your class is meant to do and how you've encapsulated that behaviour.

Is it meant to generate a random string 8 characters in length? In which case you definitely don't want to compare against the StringLength variable, as if you change it your test will still pass even though your class no longer fits it's purpose.

If it is in fact meant to generate a string of StringLength characters, then why don't you give control of that variable to the caller? For example, by passing it in as a constructor argument. This will reduce bugs in other parts of your code that may be depending on this returning a string of a certain length and give you more flexibility in using it in the future.

A well-designed class should "never" require using @TestVisible.

Related Topic