Skip to content

Add new base type handler for using nullable JDBC APIs #1243

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

kazuki43zoo
Copy link
Member

I've fixed #1242 by another approach(adding an extension point on BaseTypeHandler and adding extension class).

@harawata @mnesarco Which is better?

@kazuki43zoo kazuki43zoo force-pushed the gh-1242_add-abstract-type-handler-for-nullable-apis branch from 2d47773 to b36cb7e Compare April 5, 2018 06:17
@harawata
Copy link
Member

harawata commented Apr 5, 2018

I guess you are trying to improve other type handlers.
I think it's a good idea, but the implementation looks overly complicated for its purpose.

We should simply remove wasNull() call from the base type handler, probably [1].

public abstract class BaseTypeHandler<T> extends TypeReference<T> implements TypeHandler<T> {

  // setParameter() is as-is

  @Override
  public T getResult(ResultSet rs, String columnName) throws SQLException {
    T result;
    try {
      result = getNullableResult(rs, columnName);
    } catch (Exception e) {
      throw new ResultMapException("Error attempting to get column '" + columnName + "' from result set.  Cause: " + e, e);
    }
  }

  // pretty much the same
}

Most type handler implementations may stay unchanged and those who need wasNull() can check the result before returning the result.
Here are a few examples.

public class IntegerTypeHandler extends BaseTypeHandler<Integer> {

  // setNonNullParameter() is as-is

  @Override
  public Integer getNullableResult(ResultSet rs, String columnName)
      throws SQLException {
    int result = rs.getInt(columnName);
    return result == 0 && rs.wasNull() ? null : Integer.valueOf(result);
  }
  
  // pretty much the same
}
public class BooleanTypeHandler extends BaseTypeHandler<Boolean> {

  // setNonNullParameter() is as-is

  @Override
  public Boolean getNullableResult(ResultSet rs, String columnName)
      throws SQLException {
    boolean result = rs.getBoolean(columnName);
    return result ? Boolean.TRUE : (rs.wasNull() ? null : Boolean.FALSE);
  }

  // same
}

We don't need the extra type handler anymore and it has better reusability (e.g. subclasses can return non-null value when the column is NULL).

[1] I think it's OK to change the existing BaseTypeHandler even though it might surprise a few users. Alternatively, we can add a new base type handler and use it as a parent of all internal type handlers, but I don't like this idea very much.

@kazuki43zoo
Copy link
Member Author

@harawata OK. I agree with your opinion. I will submit new PR at later.

@kazuki43zoo kazuki43zoo closed this Apr 5, 2018
@kazuki43zoo kazuki43zoo deleted the gh-1242_add-abstract-type-handler-for-nullable-apis branch April 5, 2018 10:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants