castor

Refactoring SQLStatementStore

Details

  • Type: Improvement Improvement
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 1.3
  • Fix Version/s: 1.3.1
  • Component/s: JDO queries
  • Labels:
    None
  • Number of attachments :
    0

Description

The aim of this refactoring is to improve and restructure the implementation of SQLStatementStore. The step proposed for refactoring are:

1) Moving the SQLFieldInfo[] fields= _engine.getInfo() to the constructor and make it a class attribute because it is being used by different methods.

2) Moving SQLColumnInfo[] ids = _engine.getColumnInfoForIdentities() to the constructor and make it a class attribute because it is being used by different methods.

3) Separates the PrepareStatement from execute statement. Construct a new method that only deals with preparing statement and then call this method from within executestatement. This way the code should be more structured and modular as well.

4) Moving Identity size check from SQLStatementStore to SQLEngine.

5) Move the Storage of extended classes to the ClassMolder

Regards, Ahmad

There are no Sub-Tasks for this issue.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Ralf Joachim added a comment -

Ahmad, wouldn't it make sense to move the whole check for the failure reason of update statement into an own class. This would move everything related to _statementLoad into an own class similar to SQLStatementLookup. In my opinion would SQLSatementStore be much easier to understand thereafter.

If you agree, please create a new subtask for this.

Having said that this should be next step in my opinion.

Show
Ralf Joachim added a comment - Ahmad, wouldn't it make sense to move the whole check for the failure reason of update statement into an own class. This would move everything related to _statementLoad into an own class similar to SQLStatementLookup. In my opinion would SQLSatementStore be much easier to understand thereafter. If you agree, please create a new subtask for this. Having said that this should be next step in my opinion.
Hide
Permalink
Ralf Joachim added a comment -

In addition we may not need the full load statement for the failure check as we only need to know if there is an object in the table to update. Which then would be another subtask to work on.

Show
Ralf Joachim added a comment - In addition we may not need the full load statement for the failure check as we only need to know if there is an object in the table to update. Which then would be another subtask to work on.
Hide
Permalink
Ralf Joachim added a comment -

Ahmad, do you have any idea how we could further improve things?

Show
Ralf Joachim added a comment - Ahmad, do you have any idea how we could further improve things?
Hide
Permalink
AHMAD HASSAN added a comment -

I think it would make sense to rename SQLStatementStore to SQLStatementUpdate and move it to org.castor.cpa.persistence.sql.engine package along side SQLStatementDelete.

I created the task and attached the patch.

Regards, Ahmad

Show
AHMAD HASSAN added a comment - I think it would make sense to rename SQLStatementStore to SQLStatementUpdate and move it to org.castor.cpa.persistence.sql.engine package along side SQLStatementDelete. I created the task and attached the patch. Regards, Ahmad
Hide
Permalink
Ralf Joachim added a comment -

Any more ideas?

Show
Ralf Joachim added a comment - Any more ideas?
Hide
Permalink
Ralf Joachim added a comment -

Intend to add some minor code improvements and fix remaning checkstyle warnings.

Show
Ralf Joachim added a comment - Intend to add some minor code improvements and fix remaning checkstyle warnings.
Hide
Permalink
Ralf Joachim added a comment -

After some improvements to javadoc we can close this issue.

Show
Ralf Joachim added a comment - After some improvements to javadoc we can close this issue.

People

  • Assignee:
    Ralf Joachim
    Reporter:
    AHMAD HASSAN
Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: