Using method which return a String or creating wrapper for String

Question

Which approach is better and why?

First approach (inner class for wrapping):

public class OOP {
    private SomeService someService;

    private final class OrganizationPrimaryKeyString implements Serializable {
        private static final long serialVersionUID = -256615521081939636L;
        private final String pkString

        public OrganizationPrimaryKeyString(final OrganizationEnum organizationEnum) {
            final PK pk = someService.getValue(organizationEnum).getPk();

            if (pk == null) {
                throw new IllegalArgumentException("Can't be null");
            }

            pkString = pk.getLongValueAsString();
        }

        @Override
        public String toString() {
            return pkString;
        }
    }

    public List<Information> getInformation(final OrganizationEnum organizationEnum) {
        final OrganizationPrimaryKeyString organization = new OrganizationPrimaryKeyString(organizationEnum);

        try {
            final SessionContext ctx = createAndSetupLocalSessionContext(organization);

            (...)

            return (...);
        } catch (final FactoryException e) {
            throw new SystemException(e.getMessage(), e);
        } finally {
            closeLocalSessionContext();
        }
    }

    private SessionContext createAndSetupLocalSessionContext(final OrganizationPrimaryKeyString organization) {
        (...)
        sessionContext.setAttribute("organization", salesOrganization.toString());

        return sessionContext;
    }
}

Second approach (class method):

public class OOP {
    private SomeService someService;

    public List<Information> getInformation(final OrganizationEnum organizationEnum) {
        try {
            final SessionContext ctx = createAndSetupLocalSessionContext(organizationEnum);

            (...)

            return (...);
        } catch (final FactoryException e) {
        throw new SystemException(e.getMessage(), e);
        } finally {
            closeLocalSessionContext();
        }
    }

    private SessionContext createAndSetupLocalSessionContext(final OrganizationEnum organizationEnum) {
        (...)
        sessionContext.setAttribute("organization", getPkString(organizationEnum));

        return sessionContext;
    }

    private String getPkString(final OrganizationEnum organizationEnum) {
        final PK pk = someService.getValue(organizationEnum).getPk();

        if (pk == null) {
            throw new IllegalArgumentException("Can't be null");
        }

        return pk.getLongValueAsString();
    }
}

Answer 1

I think the first approach is the best; it uses more design-patterns:

1. You throw an IllegalArgumentExcaption but pk is not an argument of the method.

   From JavaDoc:

   Thrown to indicate that a method has been passed an illegal or inappropriate argument.

2. You throw an IllegalArgumentExcaption because of a null. It should be a NullPointerException - it's more meaningful. Objects.requreNonNull() does it for you automatically. A NullPointerException would be automatically thrown in 2 lines later, so no need for a check except you would like to add a custom message to the NullPointerException. That would be a good idea.
3. The Class who has the .getPk()-method can have a null as a PrimaryKey; it looks like a design-flaw? Anyway, if this instance who has the .getPk()-method can have an illegal state, then an IllegalStateException should be thrown.
4. The toString() implementation is good.
5. OrganizationPrimaryKeyString is a good read-only object. Read-only objects are good for memory because you do not need to have two instances of the same organisation. But if you call getInformation() one million times for ibm, then every call to getInformation() returns one million equal ibm-objects that are not the same but require its own memory in the heap! A HashMap may help caching, so every call for ibm can return one of the same object.
6. The naming is good and meaningful. getInformation() should be named getInformations() and OrganizationPrimaryKeyString should only be named OrganizationPrimaryKey.
7. OrganizationPrimaryKey should be static because YDKJ: A subclass that is not static automatically has the wrapped-class as constructor-argument who internally is cached as a field of OrganizationPrimaryKey. Trust me, every Constructor of OrganizationPrimaryKey caches the OOP-Instance.