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

Re: Suggestion/ideas for refactoring? (some design approach Qs too)

Expand Messages
  • Paul Michali
    I m a little stuck right now, not sure which way to alter my class structure. Currently I have these (major) classes: Scout - Controls the overall checking of
    Message 1 of 4 , Oct 2, 2001
    • 0 Attachment
      I'm a little stuck right now, not sure which way to alter my class
      structure. Currently I have these (major) classes:

      Scout - Controls the overall checking of paths and sending notices.
      Contains a list of PathSegments, a list of previous
      ScoutingReports, and a Dispatcher that has a list of observers
      and a message to send to them.

      PathSegment - Holds information about which web page has the road info
      we want and where to look on that page (keywords). Has
      the ability to acquire the raw HTML data. It also holds
      filter strings that are used to remove unimportant text
      from status messages for the road segment.

      ScoutingReport - Derived from BasicReport. Has several functions that
      parse through the raw HTML data to obtain the road
      condition and status message information.

      BasicReport - Abstract class that holds the road condition and status
      message for a (processed) web page. Also has routines
      to compare the condition information with other reports.

      ------------------------------------------------------------------------------
      So right now, the Scout controls the whole show. It has a list of
      PathSegments and will check the primary (first) one. It creates a
      new report, compares it to other reports, updates the notification,
      and sends out the notification. It uses the other objects to fulfill
      its goals.

      I'm not real crazy about the overall class structure right now. I
      can see numerous ways to push around the intelligence of these classes.
      For example, having the PathSegment be responsible for obtaining and
      holding reports and evaluating them. Or creating a new PathEvaluator
      object that is responsible for comparing path reports. Or having the
      reports be able to compare themselves against other reports. The
      question is, how do I figure out which way to go, without trying
      them all? Also, since this could loosely be considered legacy
      (existing) code, how does test first fit into all this?

      With the current design, there are several things that bother me.
      First, the PathSegment obtains the raw HTML information, the Report
      processes the page, but uses the filters from the PathSegment (the
      filter is for a PathSegment, but is used by the report). Seems like
      I have logic in the wrong place?

      Another is that the Scout has the high level logic for determining
      what to do based on the condition of the reports, but it uses the
      BasicReport (portion of ScoutingRepor) to do all the lower level
      checking. Should all the logic be together?

      A third is that I cannot easily test some of the methods in Scout,
      as they rely on some state information (the previous report) that
      is stored in a list that I don't have access to, unless I make calls
      just for testing, which sounds like a smell to me. Should I do
      something about Scout being a container and a "monitor"?

      Forth, I'm wondering if I should collapse the BasicReport and the
      (derived) ScoutingReport into one class. I think the only use for
      the BasicReport was so that I could create a MockScoutingReport for
      testing purposes. Any thoughts on this?

      So, I'm just not sure which way to try to evolve these classes and
      I have some concern about the responsibilities of each of the classes.
      Any comments or suggestions?


      ------------------------------------------------------------------------------
      Here is some backup info on the classes, in case it might be useful...

      Major API calls for Scout

      public BasicReport getLatestReportFor(int whichOne)
      public BasicReport getPreviousReportFor(int whichOne)
      public void makeObservations()
      public void checkAllPathSegments()
      public void reportAnyChangeInPaths()
      public void addValidReports()
      public void addReportToNotification(BasicReport report, int whichPath)
      public boolean messageChangeToReport(BasicReport current, BasicReport
      previous)
      public boolean validMessage(BasicReport current)
      public void saveAsPreviousReport(BasicReport report, int whichOne)


      Major API calls for PathSegment

      public PathSegment(String id, String route, String segment)
      public String filter(String rawStatusMsg)
      public ArrayList observe()


      Major API calls for ScoutingReport

      public ScoutingReport(PathSegment segment)
      public boolean acquire()
      public void findConditions()


      Major API calls for BasicReport

      public BasicReport(PathSegment segment)
      public boolean conditionNowOK(BasicReport previous)
      public boolean conditionStillOK(BasicReport previous)
      public boolean conditionStillBad(BasicReport older)
      public boolean conditionChanged(BasicReport previous)
      public boolean messageChanged(BasicReport previous)


      PCM (Paul Michali)

      Carrier Voice Gateway Business Unit (CVGBU)
      Cisco Systems, Inc.
      250 Apollo Drive
      Chelmsford, MA 01824

      Phone : (800) 572-6771 x45817 (978) 244-5817 [direct]
      Paging: (800) 365-4578 [voice] pcm@... [email page]
    • Bill Wake
      ... I think it s easy to feel stuck when you know a lot of stuff needs doing. I take solace in the security of refactoring: the code always works as well as it
      Message 2 of 4 , Oct 2, 2001
      • 0 Attachment
        --- In extremeprogramming@y..., Paul Michali <pcm@c...> wrote:
        > I'm a little stuck right now, not sure which way to alter my class
        > structure.

        I think it's easy to feel stuck when you know a lot of stuff needs
        doing. I take solace in the security of refactoring: the code always
        works as well as it did, and if I've addresed a real smell I know the
        whole thing is better.

        > Currently I have these (major) classes:
        >
        > Scout - Controls the overall checking of paths and sending notices.
        > Contains a list of PathSegments, a list of previous
        > ScoutingReports, and a Dispatcher that has a list
        > of observers and a message to send to them.
        >
        > [...]
        > So right now, the Scout controls the whole show. It has a list of
        > PathSegments and will check the primary (first) one. It creates a
        > new report, compares it to other reports, updates the notification,
        > and sends out the notification. It uses the other objects to fulfill
        > its goals.

        This sounds like a lot of stuff to know and do.


        > I'm not real crazy about the overall class structure right now. I
        > can see numerous ways to push around the intelligence of
        > these classes.

        You know something's not right but you don't know how to fix it.
        That's a very familiar feeling. When I get in this space, I try to
        move over and tackle some other area.

        > With the current design, there are several things that bother me.
        > First, the PathSegment obtains the raw HTML information, [...]
        > Another is that the Scout has the high level logic for determining
        > what to do based on the condition of the reports, but [...]
        > Should all the logic be together?
        (Might be helpful)

        > A third is that I cannot easily test some of the methods in Scout,
        > as they rely on some state information (the previous report) that
        > is stored in a list that I don't have access to, unless I make calls
        > just for testing, which sounds like a smell to me. Should I do
        > something about Scout being a container and a "monitor"?
        I would look at this problem intently. I'll sometimes add "test-only"
        methods, but needing them can be a sign that something else is
        missing.

        > Forth, I'm wondering if I should collapse the BasicReport and the
        > (derived) ScoutingReport into one class. I think the only use for
        > the BasicReport was so that I could create a MockScoutingReport for
        > testing purposes. Any thoughts on this?

        I wouldn't give up the ability to have a mock object if it's
        supporting your testing. Can you collapse the hierarchy and let the
        mock be a subclass of ScoutingReport? Does collapsing it clarify
        anything?

        >
        > So, I'm just not sure which way to try to evolve these classes and
        > I have some concern about the responsibilities of each of
        > the classes.
        This is always the hard part for me. I just have to fix the stuff I
        know is wrong and gnaw on the hard part in the back of my mind.

        > Any comments or suggestions?
        The problem of current vs. previous seems like it dominates a lot of
        the API. I have a feeling if you find a way to cleanly handle that,
        several things will get simpler.

        The other thing I see in your API is a lot of "int" and "String"
        parameters. Sometimes looking for the object behind them can trigger
        the right shift in responsibilities.

        Regards,
        --
        Bill Wake William.Wake@... www.xp123.com
      • Jim Murphy
        A few suggestions: 1) The Scout object seems to be doing too much fetching of data and making decisions. A little more Tell, Don t Ask approach seems in
        Message 3 of 4 , Oct 2, 2001
        • 0 Attachment
          A few suggestions:

          1) The Scout object seems to be doing too much fetching of data and making
          decisions. A little more "Tell, Don't Ask" approach seems in order. Push
          some of the work this class is doing to lower levels. This should help with
          the code smell from too much static data.

          2) It seems to me that the classes have too many responsibilities and need
          to be broken up. Use the old rule that you should be able to identify the
          class's responsibilities in 25 words or less without using conjunctions
          (like and, or, nor). Smaller, more focused classes are easier to test and
          easier to refactor.

          3) Don't be afraid to use patterns while refactoring. Processing the change
          in state of a PathSegment screams out Observer pattern. And the centralized
          processing for determining the best route make me think of using the
          Mediator pattern.

          4) I would probably have a Path class which is made up of PathSegments (as
          it appears a segment can be on more than one path) and in which no
          distinction between primary and secondary is made. Then I'd have a
          PathComparator object which could indicate a preference between any two
          Paths. Then I'd have a PathOptimizer to rank the Paths or optimize over the
          set of possible Paths.

          5) Consider the possibility that your Metaphor is no longer appropriate (now
          that you have more information and feedback from your code). Since you are
          looking for the optimal traffic flow, consider switching to a metaphor like
          a river or blood flowing through veins/capilaries.

          Regards,
          Jim
          murphyjr@...
        • Adam Judson
          ... I don t like Scout. This design seems to mimic the real world very closely, which is a good thing, but the consequence, as it often is when we model
          Message 4 of 4 , Oct 2, 2001
          • 0 Attachment
            --- In extremeprogramming@y..., Paul Michali <pcm@c...> wrote:
            > I'm a little stuck right now, not sure which way to alter my class
            > structure. Currently I have these (major) classes:
            >
            > Scout

            I don't like Scout. This design seems to mimic the real world very
            closely, which is a good thing, but the consequence, as it often
            is when we model inanimate objects, is a god class.

            In the same way, if we were to model a fruit stand, we might be
            tempted to create an owner class, that knows how to weigh/price
            each type of fruit.

            I think you should throw away the scout. The only reason you need
            him in the real world is because the road segments can't talk.

            > PathSegment

            You obviously need this information, but maybe it should be kept
            closer to the information about primary/secondary paths, and the
            road conditions?

            > ScoutingReport

            I think you should keep the final information where you make the
            decision about what info to report.

            You also keep talking about multiple paths. You have primary, and
            several secondaries. This is true, but I think it is making it
            harder to see where to put the "find me the best path" logic.

            I think you only have one path. The path you should take home. This
            path CONTAINS several choices, but in the end there is only one path.
            Now it seems obvious that this object should know what the best
            route is right now.

            In terms of testing, I can see two places that will cause some
            problems. Getting the data from the HTML page, and sending the
            e-mail message. If I remember correctly, you already have the
            e-mail portion covered. That leaves the HTML.

            My advise here is: keep it seperate from the path objects. Have an
            agent, or path-segment objects that retrieve AND parse HTML. Have
            them provide output in a standard format that your path objects
            can understand (e.g. a hash table of strings - ROAD1, GOOD, ROAD2,
            FAIR etc). Now it should be easy to create a static hash table to
            pass to your path objects to test.

            I wouldn't keep the HTML, or pass it around to other objects in the
            system, I would translate it and throw it away.

            In terms of a metaphor, how about a radio station. There's you
            listening from your car. There's the announcer who checks with the
            guy in the traffic copter every now and then. That seems to correctly
            model the two places we do communication, and if we make the
            announcer a friend of yours, then he can know about where you
            drive, and tailor the information just for you.

            This also makes me wonder about the messages your sending. I thought
            you were sending road conditions. Is this what you want, or should
            you just send the optimal path?

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