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

        Activity

        Hide
        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
        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
        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
        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
        Ralf Joachim added a comment -

        Any more ideas?

        Show
        Ralf Joachim added a comment - Any more ideas?
        Hide
        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
        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
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: