Details

    • Type: Task Task
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.4
    • Component/s: Development
    • Security Level: Public (Public: Anyone can view)
    • Labels:
      None
    • Similar issues:
      KULRICE-10110Cleanup on copy methods
      KULRICE-10052Cleanup parseExpression method
      KULRICE-12958xapool cleanup
      KULRICE-10998Cleanup: Improve cleanup scripts so that all sample data is removed from bootstrap datasets
      KULRICE-13138Apply the new deep copy method to Rice objects
      KULRICE-6864Cleanup of UIF footer beans
      KULRICE-11659Additive Database Structure: Testing and code cleanup
      KULRICE-9973General JavaDoc cleanup in the new metadata objects code
      KULRICE-7581Cleanup from Master/Detail Code Review
      KULRICE-6487Agenda Copy moves Agenda Items from old agenda to new agenda
    • Rice Module:
      KRAD
    • KRAD Feature Area:
      UIF Component
    • KAI Review Status:
      Not Required
    • KTI Review Status:
      Not Required
    • Code Review Status:
      Not Required
    • Include in Release Notes?:
      Yes

      Description

      After 2.3, I would like to do some further cleanup. One issue is our copy method with the generics is not working as planned. One of the goals of using the generics is so you don’t have to cast the type for each setter. For example instead of having to do this:

      groupCopy.setDisclosure((Disclosure) this.disclosure.copy());

      We want to be able to just do this:

      groupCopy.setDisclosure(this.disclosure.copy());

      Other cleanup (post 2.3):

      • Only need one public <T> T copy() { per hierarchy. Since this is in DictionaryBeanBase, not needed in any subclass (Component, LayoutManager, etc).

      • Create Copyable interface in org.kuali.rice.krad.datadictionary. Make DictionaryBeanBase an other objects with the copy method (not subclasses) implement

      • ‘replacement’ property on PropertyReplacer needs to be coped. Could be a component, List of components, Map of components, or Layout manager. Need to check for interface

      • Create helper methods in ComponentUtils for copying a List of Copyables and Map of Copyables so logic does not need to be duplicated everywhere, then change statements in copy properties methods to use the new helper methods

      • For nested objects that we are setting directly (not calling copy), you don’t need the null check before (just can set the value as null). This will shorten our copy methods a bit

      • Copy properties method does not use @see comment consistently, and sometimes method is after validate method

      • Formatting wise I would rather see these lines broken up:

      super.copyProperties(component);
      ContainerBase containerBaseCopy = (ContainerBase) component;
      containerBaseCopy.setDefaultItemPosition(this.defaultItemPosition);

      to

      super.copyProperties(component);
      // blank line
      ContainerBase containerBaseCopy = (ContainerBase) component;
      // blank line
      containerBaseCopy.setDefaultItemPosition(this.defaultItemPosition);

        Activity

        Hide
        Eric Westfall added a comment -

        Jerry, for what you are trying to do, i'm not sure that you even need to use generics. Couldn't you just use covariant return types? In that case on your super-most class you would just have:

        public Object copy() ...

        And in the subclasses you could override the method as:

        public Disclosure copy() ...

        Or whatever the subtype happened to be.

        Show
        Eric Westfall added a comment - Jerry, for what you are trying to do, i'm not sure that you even need to use generics. Couldn't you just use covariant return types? In that case on your super-most class you would just have: public Object copy() ... And in the subclasses you could override the method as: public Disclosure copy() ... Or whatever the subtype happened to be.
        Hide
        Jerry Neal (Inactive) added a comment -

        Eric,

        That might be a better way to do it. What we were trying is having two methods, and only child classes would need to override the second one.

        So ComponentBase would implement copy, it would create an instance of this.getClass and call the protected method copyProperties(copy). Subclasses then override copyProperties(copy) and add the necessary copy statements for that subclass.

        Trying to think through if it would be possible to just have one method. The problem I see with that is how would you deal with the creation of the copy instance. Say the subclass had:

        public Disclosure copy()

        { Disclosure copy = new Disclosure(); super.copy(); // will create its own component, we need to pass ours instead so seems like we need two methods? }

        So to do this each subclass would have the copy method like this:

        public Disclosure copy()

        { Disclosure copy = new Disclosure(); copyProperties(copy); }

        and then the copyProperties method. It would be another method for each subclass. Not a big deal but if we could somehow avoid it using generics that would be preferred.

        Maybe I am missing something?

        thanks,
        Jerry

        Show
        Jerry Neal (Inactive) added a comment - Eric, That might be a better way to do it. What we were trying is having two methods, and only child classes would need to override the second one. So ComponentBase would implement copy, it would create an instance of this.getClass and call the protected method copyProperties(copy). Subclasses then override copyProperties(copy) and add the necessary copy statements for that subclass. Trying to think through if it would be possible to just have one method. The problem I see with that is how would you deal with the creation of the copy instance. Say the subclass had: public Disclosure copy() { Disclosure copy = new Disclosure(); super.copy(); // will create its own component, we need to pass ours instead so seems like we need two methods? } So to do this each subclass would have the copy method like this: public Disclosure copy() { Disclosure copy = new Disclosure(); copyProperties(copy); } and then the copyProperties method. It would be another method for each subclass. Not a big deal but if we could somehow avoid it using generics that would be preferred. Maybe I am missing something? thanks, Jerry

          People

          • Assignee:
            Matthew Wuertz (Inactive)
            Reporter:
            Jerry Neal (Inactive)
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Structure Helper Panel