Comment on revision r2513 in power-matchmaker

[power-ma...@googlecode.com]

mo.j...@gmail.com commented on revision r2513 in project power-matchmaker.
Details are at
http://code.google.com/p/power-matchmaker/source/detail?r=2513

Line-by-line comments:

File:
/branches/spobject-conversion/src/ca/sqlpower/matchmaker/CachableTable.java
(r2513)
===============================================================================

Line 149: public void setSPDataSource(String spDataSourceName) {
-------------------------------------------------------------------------------
This is not this particular commit's fault, but this method name needs to
reflect what it's setting. It should be called setSPDataSourceName.

Line 273: firePropertyChange(propertyName, oldValue, newValue);
-------------------------------------------------------------------------------
Does it make sense that a NonProperty fires a property change event?

File:
/branches/spobject-conversion/src/ca/sqlpower/matchmaker/MMRootNode.java
(r2513)
===============================================================================

Line 72: public boolean isRoot() {
-------------------------------------------------------------------------------
Not this particular commit's fault, but this method looks like a waste of
code, and furthermore, Eclipse indicates that it is never used.

File:
/branches/spobject-conversion/src/ca/sqlpower/matchmaker/MatchMakerTranslateWord.java
(r2513)
===============================================================================

Line 64: firePropertyChange("from", oldValue, this.from);
-------------------------------------------------------------------------------
Does it make sense that a NonProperty is firing a property change?

Line 86: firePropertyChange("to", oldValue, this.to);
-------------------------------------------------------------------------------
Same here...NonProperty but it fires a property change event?

File: /branches/spobject-conversion/src/ca/sqlpower/matchmaker/Project.java
(r2513)
===============================================================================

Line 671: firePropertyChange("type", oldValue, this.type);
-------------------------------------------------------------------------------
Is this really a NonProperty?

Respond to these comments at
http://code.google.com/p/power-matchmaker/source/detail?r=2513
--
You received this message because you starred this review, or because
your project has directed all notifications to a mailing list that you
subscribe to.
You may adjust your review notification preferences at:
http://code.google.com/hosting/settings