Skip to content

Spanner - Improve methods from some of the builder patterns for generic usages #1805

Closed

Description

@emmanuelbernard

When writing things like object mappers, the builder pattern tends to get in the way unless it has some generic input methods. Here is for example the Mutation usage I had to do

        Mutation.WriteBuilder writeBuilder = Mutation.newInsertOrUpdateBuilder( key.getTable() );
		tuplePointer.getTuple().getOperations().forEach( op -> {
			switch ( op.getType() ) {
				case PUT:
					ValueBinder<Mutation.WriteBuilder> set = writeBuilder.set( op.getColumn() );
					Object object = op.getValue();
					//TODO should we convert to Google Spanner native types here or before?
					// e.g. they have their own Date and Timestamp
					if ( object instanceof Timestamp ) {
						set.to( (Timestamp) object );
					}
					else if ( object instanceof Date ) {
						set.to( (Date) object );
					}
					else if ( object instanceof String ) {
						set.to( (String) object );
					}
					else if ( object instanceof Long ) {
						set.to( (Long) object );
					}
					else if ( object instanceof Boolean ) {
						set.to( (Boolean) object );
					}
					else if ( object instanceof Double ) {
						set.to( (Double) object );
					}
					break;
				case PUT_NULL:
				case REMOVE:
					// TODO add metadata related to the type of the column we nullify
					writeBuilder.set( op.getColumn() ).to( (String) null);
                                        break;
			}

		} );
		databaseClient.write( Arrays.asList( writeBuilder.build() ) );

One improvement could be to offer a generic to(Object) method that does the instanceof check for me.

Another possible improvement (that would not help for my case) is to offer

// ideally if types is optional that's even better
setAll(String[] columnNames, Object[] columnValues, Class[] types);

Activity

  1. added
    api: spannerIssues related to the Spanner API.
    type: feature request‘Nice-to-have’ improvement, new feature or different behavior or design.
    on Mar 23, 2017
  2. saturnism commented on Apr 8, 2017

    @saturnism
  3. vkedia commented on Apr 20, 2017

    @vkedia

    @emmanuelbernard @saturnism I think this is a reasonable request. We can add a .to(Object) to the ValueBinder class. One hitch here is how to make this method work for arrays since instanceof will not tell us the type of the iterable.

  4. emmanuelbernard commented on May 3, 2017

    @emmanuelbernard
    Author

    @vkedia right. In that case, you could make a pair of explicitly named methods like toObject(Object) toIterable(Iterable value, Class<?> type).

  5. modified the milestone: Spanner GA on May 8, 2017
  6. added
    priority: p2Moderately-important priority. Fix may not be included in next release.
    on May 30, 2017
  7. yihanzhen commented on Jan 11, 2018

    @yihanzhen
    Contributor

    The feature request is added to the backlog. The issue will be closed but is linked in the backlog and can continue to be used for comment and discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

api: spannerIssues related to the Spanner API.priority: p2Moderately-important priority. Fix may not be included in next release.type: feature request‘Nice-to-have’ improvement, new feature or different behavior or design.

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions