Skip to content

Public String constants should use more specific types or enums #130

@sfloess

Description

@sfloess

Description

Three utility classes expose public String constants that could be replaced with more type-safe alternatives like enums, or at minimum should use the @StringDef pattern for better IDE support and compile-time validation.

Affected Constants

StringUtil - Error Messages and Defaults

public static final String STRING_CANNOT_BE_BLANK = "String cannot be blank!";
public static final String DEFAULT_SEPARATOR = "";

ObjectUtil - Error Message

public static final String DEFAULT_ERROR_MSG = "Invalid value";

UrlUtil - Protocol Separator

public static final String PROTOCOL_SEPARATOR = "://";

Issues with Current Design

1. String Constants for Error Messages

StringUtil.STRING_CANNOT_BE_BLANK:

  • Only used internally by requireNonBlank(String)
  • Should be private, not public
  • Public exposure serves no purpose

ObjectUtil.DEFAULT_ERROR_MSG:

  • Generic message provides no value
  • Never actually used in the codebase
  • Should be removed or made private

2. Magic String Exposure

UrlUtil.PROTOCOL_SEPARATOR:

  • Exposes internal implementation detail
  • Users shouldn't need to construct protocol + PROTOCOL_SEPARATOR + host
  • The library provides asProtocolAndHost() for this purpose

3. Lack of Type Safety

String constants can be:

  • Mistyped without compile error
  • Passed to wrong methods
  • Changed accidentally
  • Difficult to refactor

Impact

  • Severity: Low
  • Type: API Design / Encapsulation
  • Public constants expose unnecessary implementation details
  • No type safety for what should be internal values
  • Creates unnecessary public API surface

Recommended Fixes

Option 1: Make Constants Private (Recommended)

StringUtil:

// Change from public to package-private or private
private static final String STRING_CANNOT_BE_BLANK = "String cannot be blank!";
private static final String DEFAULT_SEPARATOR = "";

ObjectUtil:

// Either make private or remove entirely (not used)
private static final String DEFAULT_ERROR_MSG = "Invalid value";

UrlUtil:

// Make private - users should use asProtocolAndHost() instead
private static final String PROTOCOL_SEPARATOR = "://";

Option 2: Document Why They're Public

If these constants are intentionally public for user consumption:

/**
 * Default error message for blank string validation.
 * 
 * <p>This constant is public to allow users to recognize the exception
 * message when thrown by {@link #requireNonBlank(String)}.
 * 
 * @see #requireNonBlank(String)
 */
public static final String STRING_CANNOT_BE_BLANK = "String cannot be blank!";

But this is not recommended because:

  • Exception messages should not be part of the API contract
  • Error messages can change in future versions
  • Users shouldn't parse exception messages

Option 3: Remove Unused Constants

ObjectUtil.DEFAULT_ERROR_MSG appears to be unused:

# Search reveals no usage
grep -r "DEFAULT_ERROR_MSG" src/main/java
# Returns only the declaration

Should be removed entirely.

Analysis of Each Constant

StringUtil.STRING_CANNOT_BE_BLANK

Current usage:

public static String requireNonBlank(final String string) {
    return requireNonBlank(string, STRING_CANNOT_BE_BLANK);
}

Recommendation: Make private

  • Only used as default parameter
  • No benefit to public exposure
  • Users don't need to reference this

StringUtil.DEFAULT_SEPARATOR

Current usage:

public static StringBuilder concat(final StringBuilder stringBuilder, Object... objs) {
    return concatWithSeparator(stringBuilder, DEFAULT_SEPARATOR, objs);
}

Recommendation: Make private or replace with empty string literal

  • Empty string is self-documenting
  • No need for named constant
  • "" is clearer than DEFAULT_SEPARATOR

ObjectUtil.DEFAULT_ERROR_MSG

Current usage: None found

Recommendation: Remove entirely

  • Dead code
  • Generic message has no value
  • Not referenced anywhere

UrlUtil.PROTOCOL_SEPARATOR

Current usage:

public static String asProtocolAndHost(final URL url) {
    return StringUtil.concat(url.getProtocol(), PROTOCOL_SEPARATOR, url.getHost());
}

Recommendation: Make private

  • Internal implementation detail
  • Users should use asProtocolAndHost() method
  • No valid reason to expose "://" as public API

Breaking Change Consideration

Changing these from public to private is a breaking change per semver. However:

  1. Low risk - These constants are unlikely to be used externally
  2. Easy migration - Users can define their own if needed
  3. Better design - Encapsulates implementation details
  4. v2.0 opportunity - Could be removed in next major version

Alternative: Deprecate First

Mark as deprecated in current version, remove in v2.0:

/**
 * @deprecated Internal constant, will be removed in v2.0
 */
@Deprecated(since = "1.32", forRemoval = true)
public static final String STRING_CANNOT_BE_BLANK = "String cannot be blank!";

Metadata

Metadata

Assignees

No one assigned

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions