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

[XP] 100 Line Refactoring

Expand Messages
  • Joshua Kerievsky
    Mats, After spending 20 minutes looking over your code, my first thought is there is way too much embedded SQL in this code - it is hard to read, hard to
    Message 1 of 2 , Mar 29, 2000
    • 0 Attachment
      Mats,

      After spending 20 minutes looking over your code, my first thought is
      "there is way too much embedded SQL in this code - it is hard to read, hard
      to maintain and I want to get rid of it." Next thought: "parameterized
      SQL" For example, here are the first two queries from case
      UserID.ORGANIZATION - I've parameterized them with ^xyz^ variables.

      // Possible Query Name: AllOrganizations
      SELECT organization_id
      FROM Organization
      WHERE parent_org_id= ^parent^
      AND is_available='T'
      AND UPPER(organization_name) BETWEEN ^firstName^ AND ^lastName^
      ORDER BY UPPER(organization_name)

      // Possible Query Name: OrgsAndOthers
      SELECT organization_id
      FROM Organization
      WHERE parent_org_id= ^parent^
      AND is_available='T'
      AND UPPER(organization_name) >= ^firstName^
      ORDER BY UPPER(organization_name)

      You store these paramaterized SQL string wherever you like - in a database,
      in objects, whatever. The point is, you have the SQL strings which need to
      be filled in with values. You place your variables in a hashtable. This
      means the variable "parent, firstname, lastname, etc" go into a hashtable,
      keyed by their names. Then you write a small routine to substitute ^
      variables (in the SQL) for the real values, which are in the hashtable.
      This is a generic routine, which can be used throughout your system.

      Once you do this, you can remove tons of code from the Switch statement.
      You still have a lot more to simplify -- esp. the fallthrough logic -- but
      now you have a much smaller function, which will make it easier to do the
      next refactorings. In addition, having done this, you can write better
      tests to see that the proper UserIDs are returned from each query.

      I'll have more tomorrow.

      --jk


      _______________________________
      Industrial Logic, Inc.
      Joshua Kerievsky, founder
      mailto:joshua@...
      http://industriallogic.com
      415-292-6266
      415-292-6267 (fax)
    • Mats Henricson
      Excellent suggestion! We re using something similar in our more recently written code. I had lots to learn about SQL when this code was written. Well, I still
      Message 2 of 2 , Mar 29, 2000
      • 0 Attachment
        Excellent suggestion! We're using something similar in
        our more recently written code. I had lots to learn about
        SQL when this code was written. Well, I still have lots
        to learn about SQL...

        Mats

        > Mats,
        >
        > After spending 20 minutes looking over your code, my first thought is
        > "there is way too much embedded SQL in this code - it is hard to read, hard
        > to maintain and I want to get rid of it." Next thought: "parameterized
        > SQL" For example, here are the first two queries from case
        > UserID.ORGANIZATION - I've parameterized them with ^xyz^ variables.
        >
        > // Possible Query Name: AllOrganizations
        > SELECT organization_id
        > FROM Organization
        > WHERE parent_org_id= ^parent^
        > AND is_available='T'
        > AND UPPER(organization_name) BETWEEN ^firstName^ AND ^lastName^
        > ORDER BY UPPER(organization_name)
        >
        > // Possible Query Name: OrgsAndOthers
        > SELECT organization_id
        > FROM Organization
        > WHERE parent_org_id= ^parent^
        > AND is_available='T'
        > AND UPPER(organization_name) >= ^firstName^
        > ORDER BY UPPER(organization_name)
        >
        > You store these paramaterized SQL string wherever you like - in a database,
        > in objects, whatever. The point is, you have the SQL strings which need to
        > be filled in with values. You place your variables in a hashtable. This
        > means the variable "parent, firstname, lastname, etc" go into a hashtable,
        > keyed by their names. Then you write a small routine to substitute ^
        > variables (in the SQL) for the real values, which are in the hashtable.
        > This is a generic routine, which can be used throughout your system.
        >
        > Once you do this, you can remove tons of code from the Switch statement.
        > You still have a lot more to simplify -- esp. the fallthrough logic -- but
        > now you have a much smaller function, which will make it easier to do the
        > next refactorings. In addition, having done this, you can write better
        > tests to see that the proper UserIDs are returned from each query.
        >
        > I'll have more tomorrow.
        >
        > --jk
        >
        > _______________________________
        > Industrial Logic, Inc.
        > Joshua Kerievsky, founder
        > mailto:joshua@...
        > http://industriallogic.com
        > 415-292-6266
        > 415-292-6267 (fax)
        >
        > ------------------------------------------------------------------------
        > To Post a message, send it to: extremeprogramming@...
        > To Unsubscribe, send a blank message to: extremeprogramming-unsubscribe@...
        > Ad-free courtesy of objectmentor.com
        >
        > ------------------------------------------------------------------------
        > eGroups.com Home: http://www.egroups.com/group/extremeprogramming/
        > http://www.egroups.com - Simplifying group communications
      Your message has been successfully submitted and would be delivered to recipients shortly.