Loading ...
Sorry, an error occurred while loading the content.
 

ABILITY Output Token - Several member variables that we don't need?

Expand Messages
  • Martijn Verburg
    Hi all, The AbilityToken class has several member variables that don t appear to be of any real use. Can anyone elaborate? /** The cached PC */ private
    Message 1 of 6 , Aug 19 7:02 AM
      Hi all,

      The AbilityToken class has several member variables that don't appear to be of any real use. Can anyone elaborate?

      /** The cached PC */
      private PlayerCharacter cachedPC = null;

      /** TODO */
      private int cachedPcSerial = 0;

      /** The last token in the list of abilities */
      private String lastToken = null;

      /** The last ability category in the list of abilities */
      private AbilityCategory lastCategory = null;

      Thanks K
    • Tom Parker
      Logically, they may be of no real use, but assuming this is the same class where you got the IF statement from in the last note, it seems to me this is a cache
      Message 2 of 6 , Aug 19 8:33 AM

        Logically, they may be of no real use, but assuming this is the same class where you got the IF statement from in the last note, it seems to me this is a cache build for performance.

        Consider that a token in an output sheet like:

        ABILITY.x.NAME (making this up since there is probably a category spec in there somewhere that I've skipped)

        where X is a changing variable.  This has to query back to the token code x times, once for each ability.  If the token must reinitialize the ability list each time (in order to pull out element X from that list), then that's a rather huge performance issue in creating output.  I believe you will find many of the output tokens have this form of cache.

        TP.
        --
        Tom Parker
        thpr@... and tppublic@...

        --- On Wed, 8/19/09, Martijn Verburg <martijnverburg@...> wrote:

        From: Martijn Verburg <martijnverburg@...>
        Subject: [pcgen_developers] ABILITY Output Token - Several member variables that we don't need?
        To: pcgen_developers@yahoogroups.com
        Date: Wednesday, August 19, 2009, 10:02 AM

        Hi all,

        The AbilityToken class has several member variables that don't appear to be of any real use.  Can anyone elaborate?

        /** The cached PC */
        private PlayerCharacter cachedPC = null;

        /** TODO */
        private int cachedPcSerial = 0;

        /** The last token in the list of abilities */
        private String lastToken = null;

        /** The last ability category in the list of abilities */
        private AbilityCategory lastCategory = null;

        Thanks K




        ------------------------------------

        Yahoo! Groups Links

        <*> To visit your group on the web, go to:
            http://groups.yahoo.com/group/pcgen_developers/

        <*> Your email settings:
            Individual Email | Traditional

        <*> To change settings online go to:
            http://groups.yahoo.com/group/pcgen_developers/join
            (Yahoo! ID required)

        <*> To change settings via email:
            mailto:pcgen_developers-digest@yahoogroups.com
            mailto:pcgen_developers-fullfeatured@yahoogroups.com

        <*> To unsubscribe from this group, send an email to:
            pcgen_developers-unsubscribe@yahoogroups.com

        <*> Your use of Yahoo! Groups is subject to:
            http://docs.yahoo.com/info/terms/


      • Martijn Verburg
        Hi Tom, ... Hmm, I must be missing where that is being used in the ABILITY token then, they are private member variables and: * I don t see any sub classes of
        Message 3 of 6 , Aug 19 8:46 AM
          Hi Tom,

          > Logically, they may be of no real use, but assuming this is the
          > same class where you got the IF statement from in the last note, it
          > seems to me this is a cache build for performance.
          >
          > Consider that a token in an output sheet like:
          >
          > ABILITY.x.NAME (making this up since there is probably a category
          > spec in there somewhere that I've skipped)
          >
          > where X is a changing variable.  This has to query back to the
          > token code x times, once for each ability.  If the token must
          > reinitialize the ability list each time (in order to pull out
          > element X from that list), then that's a rather huge performance
          > issue in creating output.  I believe you will find many of the
          > output tokens have this form of cache.

          Hmm, I must be missing where that is being used in the ABILITY token then, they are private member variables and:

          * I don't see any sub classes of AbilityToken using them.
          * The class itself doesn't use them
          * I don't think reflections is involved here (extremely unlikely)
          * There are no getters/setters for these for outside classes to utilise them.

          K
        • Tom Parker
          I assume you are referring to pcgen.exporttoken.AbilityToken. Please clarify if you are not. ... Now looking at the code, AbilityToken does use them as I
          Message 4 of 6 , Aug 19 12:44 PM
            I assume you are referring to pcgen.exporttoken.AbilityToken. Please clarify if you are not.

            --- In pcgen_developers@yahoogroups.com, "Martijn Verburg" <martijnverburg@...> wrote:
            > * The class itself doesn't use them

            Now looking at the code, AbilityToken does use them as I expected; I'm not sure you're following my description of how they are used.

            They are a cache. You have to consider that this code is massively re-entrant based on how it is used in the output sheets.

            Consider the impact of this Java code:

            AbilityToken at = new AbilityToken();
            for (int i = 0; i < 10; i++)
            {
            System.out.println(at.getToken("ABILITY.Special Ability." + i + ".NAME", pc, eh));
            }

            This enters the getToken method 10 times.

            In effect, this is what happens with actual OS code like this:

            |IIF(count("ABILITIES";"CATEGORY=Special Ability";"TYPE=SpecialAttack";"VISIBILITY=DEFAULT[or]VISIBILITY=OUTPUT_ONLY")>0)| |FOR,%specialAttack,0,count("ABILITIES","CATEGORY=Special Ability","TYPE=SpecialAttack","VISIBILITY=DEFAULT[or]VISIBILITY=OUTPUT_ONLY")-1,1,0| |OIF(EVEN:%specialAttack,,)| |ENDFOR|
            SPECIAL ATTACKS
            |ABILITYALL.Special Ability.VISIBLE.%specialAttack.TYPE=SpecialAttack| |IIF(ABILITYALL.Special Ability.VISIBLE.%specialAttack.TYPE=SpecialAttack.TYPE:Extraordinary)| (Ex) |ENDIF| |IIF(ABILITYALL.Special Ability.VISIBLE.%specialAttack.TYPE=SpecialAttack.TYPE:Supernatural)| (Su) |ENDIF| |IIF(ABILITYALL.Special Ability.VISIBLE.%specialAttack.TYPE=SpecialAttack.TYPE:SpellLike)| (Sp) |ENDIF| |IIF(ABILITYALL.Special Ability.VISIBLE.%specialAttack.TYPE=SpecialAttack.TYPE:PsiLike)| (Ps) |ENDIF| |ABILITYALL.Special Ability.VISIBLE.%specialAttack.TYPE=SpecialAttack.DESC|
            |ENDIF|

            Note the %specialAttack is just like the "i" in my Java loop above... it increments inside of the FOR loop in the output sheet rather than a direct "for" loop in Java.

            Note what happens inside of AbilityToken for these loop cases:

            The first time through getToken, it passes to getTokenForCategory, and the "cache" (composed of cachedPC, lastCategory, cachedPcSerial and lastToken) is all null/empty. getAbilityList is called since the cachedPC != pc test is true.

            The second time through getToken, it passes to getTokenForCategory, but (1) the PC is the same (2) the Category is the same (3) the PC serial is the same and (4) the "lastToken" is the same (since the lastToken is actually the first item in the output token, which in this case is "ABILITY")

            As a result of the cache test "passing" (meaning all false and the IF is not entered), getAbilityList is not called the second (and i-th where i > 0) time through the loop. This is a performance enhancement for output.

            We could debate whether it's valuable in this instance, but I don't think we should be worried right now about this level of detail and code cleanup in the output tokens. I would prefer a focus on behavior vs. docs. In this case in particular, I'm not worried, since the Ability list will be maintained (and cached) by the AbilityFacet, so the output token will not have to do this type of caching in the future (which will help clarity in the output tokens)

            TP.
          • karianna03
            I ve run out of time to look at this before I go, I ll reply later.
            Message 5 of 6 , Aug 20 7:38 AM
              I've run out of time to look at this before I go, I'll reply later.

              > I assume you are referring to pcgen.exporttoken.AbilityToken. Please clarify if you are not.
              >
              > --- In pcgen_developers@yahoogroups.com, "Martijn Verburg" <martijnverburg@> wrote:
              > > * The class itself doesn't use them
              >
              > Now looking at the code, AbilityToken does use them as I expected; I'm not sure you're following my description of how they are used.
              >
              > They are a cache. You have to consider that this code is massively re-entrant based on how it is used in the output sheets.
              >
              > Consider the impact of this Java code:
              >
              > AbilityToken at = new AbilityToken();
              > for (int i = 0; i < 10; i++)
              > {
              > System.out.println(at.getToken("ABILITY.Special Ability." + i + ".NAME", pc, eh));
              > }
              >
              > This enters the getToken method 10 times.
              >
              > In effect, this is what happens with actual OS code like this:
              >
              > |IIF(count("ABILITIES";"CATEGORY=Special Ability";"TYPE=SpecialAttack";"VISIBILITY=DEFAULT[or]VISIBILITY=OUTPUT_ONLY")>0)| |FOR,%specialAttack,0,count("ABILITIES","CATEGORY=Special Ability","TYPE=SpecialAttack","VISIBILITY=DEFAULT[or]VISIBILITY=OUTPUT_ONLY")-1,1,0| |OIF(EVEN:%specialAttack,,)| |ENDFOR|
              > SPECIAL ATTACKS
              > |ABILITYALL.Special Ability.VISIBLE.%specialAttack.TYPE=SpecialAttack| |IIF(ABILITYALL.Special Ability.VISIBLE.%specialAttack.TYPE=SpecialAttack.TYPE:Extraordinary)| (Ex) |ENDIF| |IIF(ABILITYALL.Special Ability.VISIBLE.%specialAttack.TYPE=SpecialAttack.TYPE:Supernatural)| (Su) |ENDIF| |IIF(ABILITYALL.Special Ability.VISIBLE.%specialAttack.TYPE=SpecialAttack.TYPE:SpellLike)| (Sp) |ENDIF| |IIF(ABILITYALL.Special Ability.VISIBLE.%specialAttack.TYPE=SpecialAttack.TYPE:PsiLike)| (Ps) |ENDIF| |ABILITYALL.Special Ability.VISIBLE.%specialAttack.TYPE=SpecialAttack.DESC|
              > |ENDIF|
              >
              > Note the %specialAttack is just like the "i" in my Java loop above... it increments inside of the FOR loop in the output sheet rather than a direct "for" loop in Java.
              >
              > Note what happens inside of AbilityToken for these loop cases:
              >
              > The first time through getToken, it passes to getTokenForCategory, and the "cache" (composed of cachedPC, lastCategory, cachedPcSerial and lastToken) is all null/empty. getAbilityList is called since the cachedPC != pc test is true.
              >
              > The second time through getToken, it passes to getTokenForCategory, but (1) the PC is the same (2) the Category is the same (3) the PC serial is the same and (4) the "lastToken" is the same (since the lastToken is actually the first item in the output token, which in this case is "ABILITY")
              >
              > As a result of the cache test "passing" (meaning all false and the IF is not entered), getAbilityList is not called the second (and i-th where i > 0) time through the loop. This is a performance enhancement for output.
              >
              > We could debate whether it's valuable in this instance, but I don't think we should be worried right now about this level of detail and code cleanup in the output tokens. I would prefer a focus on behavior vs. docs. In this case in particular, I'm not worried, since the Ability list will be maintained (and cached) by the AbilityFacet, so the output token will not have to do this type of caching in the future (which will help clarity in the output tokens)
              >
              > TP.
              >
            • karianna03
              Hi Tom, ... ... Ah, now I get it, I was lost on the OS causing the re-entrant thing. OK, so I ve modified it to be a local check which goes
              Message 6 of 6 , Sep 8, 2009
                Hi Tom,

                > > * The class itself doesn't use them
                >
                > Now looking at the code, AbilityToken does use them as I expected;
                > I'm not sure you're following my description of how they are used.
                >
                > They are a cache. You have to consider that this code is massively
                > re-entrant based on how it is used in the output sheets.
                >
                > Consider the impact of this Java code:
                >
                > AbilityToken at = new AbilityToken();
                > for (int i = 0; i < 10; i++)
                > {
                > System.out.println(at.getToken("ABILITY.Special Ability." + i + ".NAME", pc, eh));
                > }
                >
                > This enters the getToken method 10 times.
                >
                > In effect, this is what happens with actual OS code like this:

                <snip OS code>

                > Note the %specialAttack is just like the "i" in my Java loop
                > above... it increments inside of the FOR loop in the output sheet
                > rather than a direct "for" loop in Java.
                >
                > Note what happens inside of AbilityToken for these loop cases:
                >
                > The first time through getToken, it passes to getTokenForCategory,
                > and the "cache" (composed of cachedPC, lastCategory, cachedPcSerial
                > and lastToken) is all null/empty. getAbilityList is called since
                > the cachedPC != pc test is true.
                >
                > The second time through getToken, it passes to getTokenForCategory,
                > but (1) the PC is the same (2) the Category is the same (3) the PC
                > serial is the same and (4) the "lastToken" is the same (since the
                > lastToken is actually the first item in the output token, which in
                > this case is "ABILITY")
                >
                > As a result of the cache test "passing" (meaning all false and the
                > IF is not entered), getAbilityList is not called the second (and i-
                > th where i > 0) time through the loop. This is a performance
                > enhancement for output.

                Ah, now I get it, I was lost on the OS causing the re-entrant thing. OK, so I've modified it to be a local check which goes back to a boolean called cacheAbilityProcessingData.

                > We could debate whether it's valuable in this instance, but I don't
                > think we should be worried right now about this level of detail and
                > code cleanup in the output tokens. I would prefer a focus on
                > behavior vs. docs. In this case in particular, I'm not worried,
                > since the Ability list will be maintained (and cached) by the
                > AbilityFacet, so the output token will not have to do this type of
                > caching in the future (which will help clarity in the output tokens)

                Completely Agree :)

                K
              Your message has been successfully submitted and would be delivered to recipients shortly.