Should a string constant be defined if it's only going to be used once? Please suggest why not?

Question

I am working on Java + Spring Boot example and have below code snippets.

Case-1

public class EmployeeRowMapper implements RowMapper<Employee>{

    @Override
    public Employee mapRow(ResultSet rs, int rowNum) throws SQLException {
        return Employee.builder()
                .firstName(rs.getString("FIRST_NAME"))
                .lastName(rs.getString("LASTN_AME"))
                .email(rs.getString("EMAIL"))
                .address(rs.getInt("ADDRESS")).build();
    }
}

Now, people (in code review comments) are suggesting me to create constants for FIRST_NAME, LAST_NAME, EMAIL and ADDRESS like below which I feel is absolutely not needed and doesn't bring any usefulness as per my knowledge and creating these constants will consume 4 bytes of memory and reference to these variable will never be garbage collected.

This is simple use-case I've shown using small code snippet of Spring Batch code, but this approach people do use when creating Controllers, Services or DAO/Repository layer in Java and where not.

Case-2

public class EmployeeRowMapper implements RowMapper<Employee>{
    public static final String FIRST_NAME = "FIRST_NAME";
    public static final String LAST_NAME = "LAST_NAME";
    public static final String EMAIL = "EMAIL";
    public static final String ADDRESS = "ADDRESS";

    @Override
    public Employee mapRow(ResultSet rs, int rowNum) throws SQLException {
        return Employee.builder()
                .firstName(rs.getString(FIRST_NAME))
                .lastName(rs.getString(LAST_NAME))
                .email(rs.getString(EMAIL))
                .address(rs.getInt(ADDRESS)).build();
    }
}

Note: Here I really need to understand which Case (i.e., Case-1 or Case-2) I need to follow and why ?

Also, people changing below controller to

@GetMapping(path = "/employee/{employeeId}")
public EmployeeDto getEmployee(@PathVariable Long employeeId) {
    return employeeIdService.getTridById(employeeId);
}

To

public static final String EMP_GET = "/employee/{employeeId}";

@GetMapping(path = EMP_GET)
public EmployeeDto getEmployee(@PathVariable Long employeeId) {
    return employeeIdService.getTridById(employeeId);
}

Say I've 100 Controllers and almost 500 endpoints in my modular application, so logically I am creating 500 String constants. Does this 500 constants doesnot occupy the memory area in the string constant pool?

Answer 1

There are no "reasons" to do this, unless your team has an enforcement for readability (we have the same ones in our code reviews, I'll explain why later).

These are string literals in your case, so only one instance created; I hope people providing you comments are aware of this. But even if these would not be a single instance objects, from a performance perspective - this is ridiculous... On one hand you are using spring (proxies, proxies everywhere!) and your team is worried about String instances and GC pressure because of that? Well...

Why we enforce such a rule?

• In my teams view - this is easier to read the code.

• as soon as you use one constant at least twice in the same class, things are a lot more readable (in my teams view). i.e.:

---

Employee amp = Employee.builder()
                       .firstName(rs.getString(FIRST_NAME))
                       .lastName(rs.getString(LAST_NAME))
                       .email(rs.getString(EMAIL))

// ... later in the code re-use FIRST_NAME, LAST_NAME

In case you need to change "FIRST_NAME" - you replace it a in single place.

• we provide all constants in the beginning of the class (again, this is subjective to my teams standards, may be yours does not).

What we don't do is to provide constants for spring controller mappings, it just creates too much noise and makes the path to read much more complicated.

Answer 2

In most cases, you shouldn't worry about memories. Readability in business applications is much more important

In the case with rest controllers it is easier to view and analyse all paths when they are grouped in one place instead of scattered in random places. Especially when you collect all routings in one file.

In the case with fields names, when you will name the field

private static final String ADDRESS_COLUMN_NAME = "ADDRESS";

You simply say that this is the name of the column in your database, not the field in the properties file. And of course you have it grouped in one place so you don't have to search the file.

Sometimes you can explain the weird API, lets say

String userName = response.get("benutzer").get("name");

or

private static final String USER_FIELD_IN_SOME_GERMANE_API = "benutzer";
private static final String USER_NAME_FIELD_IN_SOME_GERMANE_API = "benutzer";

String userName = response.get(USER_FIELD_IN_SOME_GERMANE_API).get(USER_NAME_FIELD_IN_SOME_GERMANE_API);

much better, isn't it?

Another example

if (error.equals("error 3214e.r") {
   // ...
}
// or
if (error.equals(USER_NOT_EXIST_ERROR_CODE) {
   // ...
}

While for me personally strings are not always a big problem, but always create variables with numbers (magic numbers)

for (int i = 0; i < 52; i++) 
   pickRandomCard()

// lol, why 52?

for (int i = 0; i < DECK_SIZE; i++) 
   pickRandomCard()