Move updateSSComponent() from combo implementations in SSBaseComboBox

[bpangburn]

Currently there are similar, but separate implementations of updateSSComponent() in SSDBComboBox and SSComboBox. They should be consolidated in SSBaseComboBox, but have to handle the different types expected or make it type-agnostic.

[bpangburn]

Somewhat related to Issue #17

[bpangburn]

Per @errael:

Is this an aspect of #17? I'd consider splitting RowSetOps.updateColumnText, something like

Object RowSetOps.parseText(JDBCTYpe type, String text)

RowSetOps.updateColumnText(...)
    _rowSet.updateObject(parseColumnText(...))

And then (M)RowSetOps.parseText(...) could work (possible with a check warning).

Adding an abstractMethod like (M)parseString is certainly a workaround.

[errael]

@bpangburn What do you think about a Object RowSetOps.parseText(JDBCTYpe type, String text) method?

I guess there would be a method RowSetOps.parseText(int columnIndex, String text) and it picks up the JDBCType from the metadata. This method would invoke the parseText(type, text) method.

This counts on the database type matching up with what's needed. BIGINT for Long in this case to get a Mapping.

If that does't work, either need to explicitly tell the combo box, or use some refection to get the proper type. (guava has some stuff to help pick up generic type info, https://stackoverflow.com/questions/3403909/get-generic-type-of-class-at-runtime).

[bpangburn]

@errael I think this is a good approach. While it does not yet use JDBCTypes, SSFormattedTextField.updateSSComponent() does something somewhat similar except that it expects the component can handle the Object type and uses getRowSet().getObject(columnName) so it doesn't cast back/forth to a String.

[errael]

Tried a few things, note in the following getMappingType() returns the class of (M)

1. M t = (M) parseText(getBoundColumnJDBCType(), boundColumnText);
   This uses the metadata as a guide for parsing the text into an object. This fails since SSDBCombo always expects a Long now matter the database type.
2. M t = (M)RowSetOps.getValueOf(getMappingType(), boundColumnText);
   This one uses the class of the mapping and if there is a valueOf(String) method, invokes it. This is similar to how the combo editor reverse maps a string to an item. Limited and unsatisfying.
3. M t = (M)parseText(classToJdbcType(getMappingType()), boundColumnText);
   Takes the class converts it to JDBCType, then parseText(type, text).

Some things of note

• getMappingType() - guava magic makes it trivial to get runtime class of generic. See SSBaseComboBox.
• classToJdbcType(clazz) - new file: datasources/JdbcDataTypeConversionTables.java
  The new class holds JDBC4.2 spec's appendix B, with two tables implemented so far; we'll see if the rest are needed. The named method presents table 4. I've been wanting to do this for a while to convince myself I have at least a small understanding of these tables.
• parseText(jdbctype, text)
  In RowSetOps.updateColumnText(...), moved the switch to parseText().

I've got an experimental branch with this (also fixes FLOAT/REAL backwards in RowSetOps).
updateSSComponent-into-SSBaseComboBox

Still tweaking, but a little something to kick off post v4.

[bpangburn]

@errael

I agree that [3] is the way to go, but want to avoid bringing in guava or any new dependencies into SS4.x. I came up with the following, which seems to work. It's along the lines of [2], but using instanceof.

Thinking of adding to 4.0.4, then we can make a pass to delete commented blocks, then we can move 4.x into a legacy branch, make SS5 master, and I can get out of the way so you can bring in the new dependencies.

	/**
	 * Updates the value stored and displayed in the SwingSet component based on
	 * getBoundColumnText()
	 * <p>
	 * Call to this method should be coming from SSCommon and should already have
	 * the Component listener removed
	 */
	@SuppressWarnings("unchecked")
	@Override
	public void updateSSComponent() {
		// TODO Modify this class similar to updateSSComponent() in SSFormattedTextField and only limit JDBC types accepted
		try {
			// If initialization is taking place then there won't be any mappings so don't try to update anything yet.
			if (!hasItems()) {
				return;
			}

			// Maybe insures blank in case of later exception.
			setSelectionPending(true);

			// SSDBComboBox will generally work with primary key column data queried from the database, which will generally be of data type long.
			// SSComboBox is generally used with 2 or 4 byte integer columns.
			final String boundColumnText = getBoundColumnText();

			// LOGGING
			logger.debug("{}: getBoundColumnText() - " + boundColumnText, () -> getColumnForLog());
			
			// GET THE BOUND VALUE STORED IN THE ROWSET - may throw a NumberFormatException
			M targetValue = null;
			if ((boundColumnText != null) && !boundColumnText.isEmpty()) {
				// https://github.com/bpangburn/swingset/issues/46
				if (this instanceof SSComboBox) {
					targetValue = (M)(Object)Integer.parseInt(boundColumnText);
				} else if (this instanceof SSDBComboBox) {
					targetValue = (M)(Object)Long.parseLong(boundColumnText);
				} else {
					throw new Exception();
				}
			}
			
			// LOGGING
			logger.debug("{}: targetValue - " + targetValue, () -> getColumnForLog());
			
			// UPDATE COMPONENT
			setSelectedMapping(targetValue);// setSelectedMapping() should handle null OK.}

		} catch (final NumberFormatException nfe) {
			JOptionPane.showMessageDialog(this, String.format(
					"Encountered database value of '%s' for column [%s], which cannot be converted to a number.", getBoundColumnText(), getColumnForLog()));
			logger.error(getColumnForLog() + ": Number Format Exception.", nfe);
		} catch (Exception e) {
			JOptionPane.showMessageDialog(this, String.format(
					"Expecting SSComboBox or SSDBComboBox, but component for column [%s] is of type %s.", this.getClass(), getColumnForLog()));
			logger.error(getColumnForLog() + ": Unknown SwingSet component of " + this.getClass() + ".", e);
		}
	}

[errael]

When I saw this last night, I why, but I couldn't see a strong reason not to make the change . This morning's wakeup flash was that this change prevents doing something like

class someCombo extends SSBaseComboBox<String, Object, Object>

which otherwise would work (AFAICT). This feels like the wrong direction. I guess the else throw Exception() does CYA, but if someone runs into it the inclination is to add an if.

Typically when there's weird if statements like this, the idea would be to push the weirdness into a subclass (not the other way around). One thought for 4.x is have an abstract getMappingType() and use the code with [2].

M t = (M)RowSetOps.getValueOf(getMappingType(), boundColumnText)

I believe that gets rid of the guava dependency.

All that said, the suggested change minimizes things to this one method which makes future cleanup easier and so would be better than the [2] suggestion with abstract method. I'd probably mention in some comment that this is not forwards compatible and the comments about SSDBComboBox/SSComboBox should be qualified as 4.x only; strange for a superclass to refer to a subclass this way. (although sealed classes, https://openjdk.java.net/jeps/360, previewed in jdk 15 change that)

A note about @SuppressWarnings("unchecked"). I picked up Effective Java in December, there's Item 27: Eliminate unchecked warnings and Always use ... smallest possible scope (it's bold in the book). Since then I've been trying to do that on any new code (should probably fixup any older cases I come across). Using method scope could let things sneak in. The book notes that sometimes requires an extra local variable.

The experimental branch updateSSComponent-into-SSBaseComboBox has examples of minimal unchecked scope and RowSetOps.getValueOf() .

[errael]

To summarize, that last comment is a long winded "go for it".

[bpangburn]

M t = (M)RowSetOps.getValueOf(getMappingType(), boundColumnText)

I tried this, but getMappingType() relies upon
import com.google.common.reflect.TypeToken;

I can certainly expand the comment to mention what is going on. If someone wrote another child class they could always override updateSSComponent(). Completely agree that your solution with new dependencies is generally preferable and should get dropped into 5. Guess my thought is to shrink the code as much as possible in 4 before launching 5 (other than deprecations). Step 1 in 5 will probably be to kill deprecations.

[errael]

One thought for 4.x is have an abstract getMappingType() and use the code with [2].

I was thinking SSDBCombo and SSCombo would override this. But that seems messy if the idea is to minimize changes going forward. And so the original suggestion seems best.

[bpangburn]

@errael Do you think we should close this and open a new item for your improved solution (3) above in SS5 with added dependencies or just leave this open?