Skip to content

Replace ResultSet-returning DbInteractions.query with a RowMapper-style API returning List<T> #144

Description

@dmccoystephenson

Summary

DbInteractions.query(...) returns a ResultSet (a disconnected CachedRowSet as of #143). Because the raw row cursor is handed back to callers, every repository re-implements the same iterate-map-and-error-handle boilerplate. A RowMapper-style API that returns List<T> would move that loop (and all SQLException/null handling) into one place and let repositories return mapped objects directly.

This is a follow-up to #143, which fixed the resource leak (#140) and parameterized the SQL (#139) with minimal blast radius but deliberately left the ResultSet-returning shape in place.

The repeated boilerplate

Each *RepositoryImpl query method follows the same pattern — e.g. src/main/java/preponderous/viron/repositories/EnvironmentRepositoryImpl.java:26-37:

List<Environment> environments = new ArrayList<>();
ResultSet rs = dbInteractions.query("SELECT * FROM viron.environment");
if (rs == null) {
    log.error("Error finding all environments: ResultSet is null");
    return environments;
}
try {
    while (rs.next()) {
        environments.add(mapResultSetToEnvironment(rs));
    }
} catch (SQLException e) {
    log.error(...);
}
return environments;

This null-guard + try/while (rs.next()) + catch (SQLException) block is repeated across ~18 query methods in the four repositories (null-guards: EnvironmentRepositoryImpl 7, EntityRepositoryImpl 6, GridRepositoryImpl 4, LocationRepositoryImpl 1), and each repository carries a near-identical mapResultSetTo* row-mapper helper:

  • EnvironmentRepositoryImpl.java:195 mapResultSetToEnvironment
  • EntityRepositoryImpl.java:23 mapResultSetToEntity
  • GridRepositoryImpl.java:23 mapResultSetToGrid
  • LocationRepositoryImpl.java:23 mapResultSetToLocation

Why it matters

Suggested approach

  1. Add a functional interface:
    @FunctionalInterface
    public interface RowMapper<T> {
        T map(ResultSet rs) throws SQLException;
    }
  2. Add DbInteractions.query overloads that own the cursor and return mapped results, e.g.:
    <T> List<T> query(String sql, RowMapper<T> mapper, Object... params);          // 0..n rows
    <T> Optional<T> queryOne(String sql, RowMapper<T> mapper, Object... params);    // 0..1 row
    These iterate, map, and close the PreparedStatement/ResultSet internally, returning an empty List/Optional on error.
  3. Migrate each repository method to pass its existing mapResultSetTo* helper (or a method reference) and return the result directly, deleting the per-method null-guard and try/catch boilerplate.
  4. Once all callers are migrated, remove the ResultSet-returning query (and the CachedRowSet machinery it needs).

This is a larger, mechanical refactor touching all four repositories and the factories, so it is intentionally separated from #143. It pairs naturally with #130 (data-layer/contract hygiene).

Filed by Claude during an automated triage pass; claims above were verified against source.

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