JSPWiki
  1. JSPWiki
  2. JSPWIKI-115

Change VariableManager.getValue to use reflection instead of a series of if statements

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.8
    • Component/s: Core & storage
    • Labels:
      None

      Description

      A fixme tag was on getValue about cleaning up how system variables were handled. Currently a series of if else blocks goes through and designates a response for specific variables names.

      1. var_reflection_wGets.patch
        11 kB
        Stephen Solka
      2. var_reflection.patch
        11 kB
        Stephen Solka

        Activity

        Hide
        Stephen Solka added a comment -

        The if else blocks have been replaced by the use of reflection. An internal private class has been created within VariableManager called SystemVariables. This class has a collection of public methods that each return a string and start with "var_". An attempt is made to obtain a method based on the variable name being referenced. If a NoSuchMethodException is thrown we then know that the variable name is not a system variable and we then try the other cases such as page variables and request headers.

        This change greatly increases the readability of the getValue method and trivializes adding new system variables.

        Show
        Stephen Solka added a comment - The if else blocks have been replaced by the use of reflection. An internal private class has been created within VariableManager called SystemVariables. This class has a collection of public methods that each return a string and start with "var_". An attempt is made to obtain a method based on the variable name being referenced. If a NoSuchMethodException is thrown we then know that the variable name is not a system variable and we then try the other cases such as page variables and request headers. This change greatly increases the readability of the getValue method and trivializes adding new system variables.
        Hide
        Stephen Solka added a comment -

        I forgot to mention patch was created against HEAD checked out december 26th. I believe its called v2.6.1-cvs-1

        Show
        Stephen Solka added a comment - I forgot to mention patch was created against HEAD checked out december 26th. I believe its called v2.6.1-cvs-1
        Hide
        Janne Jalkanen added a comment -

        Good idea, but I think it might be a better to stick to the regular getXXX() conventions of beans. They will make life easier in the future.

        Show
        Janne Jalkanen added a comment - Good idea, but I think it might be a better to stick to the regular getXXX() conventions of beans. They will make life easier in the future.
        Hide
        Stephen Solka added a comment -

        Since currently we lowercase varnames prior to processing do you think getvarname() would look odd? example getusername(). Normally with beans its getUsername(). Should I capitalize the first letter before obtaining the methods that way they stick to the bean standard or should I just change var_ to get?

        Show
        Stephen Solka added a comment - Since currently we lowercase varnames prior to processing do you think getvarname() would look odd? example getusername(). Normally with beans its getUsername(). Should I capitalize the first letter before obtaining the methods that way they stick to the bean standard or should I just change var_ to get?
        Hide
        Janne Jalkanen added a comment -

        Yup, capitalization sounds good. That's the Java standard

        Show
        Janne Jalkanen added a comment - Yup, capitalization sounds good. That's the Java standard
        Hide
        Stephen Solka added a comment -

        Changed var_ to get and and made the SystemVariables class more beany. getValue now expects to see methods as such
        public String getMysysvar()

        { return "Hello World"; }
        Show
        Stephen Solka added a comment - Changed var_ to get and and made the SystemVariables class more beany. getValue now expects to see methods as such public String getMysysvar() { return "Hello World"; }
        Hide
        Janne Jalkanen added a comment -

        Looks good. Proposing we take this in for 2.8.

        Show
        Janne Jalkanen added a comment - Looks good. Proposing we take this in for 2.8.
        Hide
        Janne Jalkanen added a comment -

        Fixed in 2.7.0-svn-8

        Show
        Janne Jalkanen added a comment - Fixed in 2.7.0-svn-8

          People

          • Assignee:
            Unassigned
            Reporter:
            Stephen Solka
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development