Torque
  1. Torque
  2. TORQUE-72

Beans should not require torque.objectIsCaching = true setting to create all complexModel methods.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 3.3
    • Fix Version/s: 4.0-beta1
    • Component/s: Generator
    • Labels:
      None

      Description

      Record Objects generated with complexModel = true will include the get all associated FK records methods, e.g.

      List books = author.getBooks()

      regardless of the objectIsCaching setting.

      Beans however, will NOT generate the get associated FK records methods unless objectIsCaching is true.

      FWIW, the get related PK object methods are generated, e.g.

      Author a = book.getAuthor();

      regardless of objectIsCaching.

      The Bean template coding needs to be reviewed and fixed to generate code correctly.

        Activity

        Hide
        Thomas Fox added a comment -

        I'm not sure whether the FK get methods make sense without complexObjectModel set to true. If one changes the value of complexObjectModel, the interface of the database objects changes, so why should the interface of the bean objects not change ?

        Considering what these methods should do if complexObjectModel is set to false makes this point clear: Currently, if complexObjectModel is set to true, the current bean creation code just takes the lists of cached related objects and stores them in the beans. If they have not been queried yet, nothing is stored, even when related objects exist.
        In the bean, one can tell the difference between "was not queried" and "no related objects exist" because the returned list is null in the former case and an empty list in the latter.

        So what should happen in the get methods if complexObjectModel is false ? I can see three possibilities
        1) query the db on bean creations for the foreign keys and store the results in the beans
        2) always return null
        3) always throw an exception

        1) is not really an option in my eyes. It is contrary to the intent of setting complexObjectModel to false.
        2) would always tell "no objects cached". If these methods are added, this would be my preferred implementation.
        3) would say "this method is only there for interface compliance reasons". This would be different to the implementation if complexObjectModel is turned on, and thus is in my eyes inferior to solution 2)

        Assuming the implementation 2, what would an application do with these methods ? If an application handles the beans only and not the full Torque objects, it has no chance whatsoever to react on the condition "the related object cache is not filled", because the bean has no access to the database and cannot fill the related objects cache istelf. Retrieving the related objects make sense only if one relies on the torque application to fill these fields, otherwise one would not call the method at all. And if a method is not called, why should it exist at all ?

        Show
        Thomas Fox added a comment - I'm not sure whether the FK get methods make sense without complexObjectModel set to true. If one changes the value of complexObjectModel, the interface of the database objects changes, so why should the interface of the bean objects not change ? Considering what these methods should do if complexObjectModel is set to false makes this point clear: Currently, if complexObjectModel is set to true, the current bean creation code just takes the lists of cached related objects and stores them in the beans. If they have not been queried yet, nothing is stored, even when related objects exist. In the bean, one can tell the difference between "was not queried" and "no related objects exist" because the returned list is null in the former case and an empty list in the latter. So what should happen in the get methods if complexObjectModel is false ? I can see three possibilities 1) query the db on bean creations for the foreign keys and store the results in the beans 2) always return null 3) always throw an exception 1) is not really an option in my eyes. It is contrary to the intent of setting complexObjectModel to false. 2) would always tell "no objects cached". If these methods are added, this would be my preferred implementation. 3) would say "this method is only there for interface compliance reasons". This would be different to the implementation if complexObjectModel is turned on, and thus is in my eyes inferior to solution 2) Assuming the implementation 2, what would an application do with these methods ? If an application handles the beans only and not the full Torque objects, it has no chance whatsoever to react on the condition "the related object cache is not filled", because the bean has no access to the database and cannot fill the related objects cache istelf. Retrieving the related objects make sense only if one relies on the torque application to fill these fields, otherwise one would not call the method at all. And if a method is not called, why should it exist at all ?
        Hide
        CG Monroe added a comment -

        Sorry to take so long to reply to this Thomas.. saw the comment but put it on hold because my company was moving to a new building and that ate up my time (include parts of Xmas break..sigh). I'm just now finding the time to digest this.

        Some additional background is that this came up in testing 3.3-RC1 against different combinations of compile options. This issue is really just a "placeholder" that there needs to be a clearer understanding of bean behaviour and matching changes to the templates/test project to ensure they works the way they are designed.

        That said, in re-reading the Beans entry in the reference, I would think the correct behaviour would be as follows:

        If complexObjectModel = false, then a bean should just have the field setters/getters and no FK methods. This is the same as the Torque record.

        If complexObjectModel = true, then a bean should have the same FK methods as the Torque record.

        This should work regardless of the objectIsCaching setting. The current templates don't do this.

        As you point out, the hard part is deciding what the correct bean behaviour should be for the objectIsCaching=true or false.

        If objectIsCaching=true, then the bean should get the current cache values.

        If objectIsCaching=false, then as you said, the methods should return null. I know that this makes the methods worthless, but it makes beans and record objects symetrical.

        Show
        CG Monroe added a comment - Sorry to take so long to reply to this Thomas.. saw the comment but put it on hold because my company was moving to a new building and that ate up my time (include parts of Xmas break..sigh). I'm just now finding the time to digest this. Some additional background is that this came up in testing 3.3-RC1 against different combinations of compile options. This issue is really just a "placeholder" that there needs to be a clearer understanding of bean behaviour and matching changes to the templates/test project to ensure they works the way they are designed. That said, in re-reading the Beans entry in the reference, I would think the correct behaviour would be as follows: If complexObjectModel = false, then a bean should just have the field setters/getters and no FK methods. This is the same as the Torque record. If complexObjectModel = true, then a bean should have the same FK methods as the Torque record. This should work regardless of the objectIsCaching setting. The current templates don't do this. As you point out, the hard part is deciding what the correct bean behaviour should be for the objectIsCaching=true or false. If objectIsCaching=true, then the bean should get the current cache values. If objectIsCaching=false, then as you said, the methods should return null. I know that this makes the methods worthless, but it makes beans and record objects symetrical.
        Hide
        Thomas Fox added a comment -

        Never mind the response time. My response time is far worse some times but then we're volunteers and not paid for this, so no need to scrape the last bit off the free time in my opinion.

        Ok, so I'd suggest the following: If complexObjectModel = true, there should be a getter, a setter and a list of objects in the bean.
        If objectIsCaching=false, this setter in the bean will not be used in data object-> bean conversion, and the getter in the bean will not be used in bean->data object conversion. So the user can fill the related objects in the bean manually, but they are not filled automatically.
        If objectIsCaching=true, the current behaviour is retained.

        This is the best solution I can think of with the foreign keys getters and setters in the bean for objectIsCaching=false.

        Show
        Thomas Fox added a comment - Never mind the response time. My response time is far worse some times but then we're volunteers and not paid for this, so no need to scrape the last bit off the free time in my opinion. Ok, so I'd suggest the following: If complexObjectModel = true, there should be a getter, a setter and a list of objects in the bean. If objectIsCaching=false, this setter in the bean will not be used in data object-> bean conversion, and the getter in the bean will not be used in bean->data object conversion. So the user can fill the related objects in the bean manually, but they are not filled automatically. If objectIsCaching=true, the current behaviour is retained. This is the best solution I can think of with the foreign keys getters and setters in the bean for objectIsCaching=false.

          People

          • Assignee:
            Thomas Fox
            Reporter:
            CG Monroe
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development