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

Re: [XP] Re: Suggestion/ideas for refactoring?

Expand Messages
  • Paul Michali
    ... Yes HTML is the source. In the terminology that I use, I call this web page data a report, because in the domain, it is a report of road conditions. I do
    Message 1 of 7 , Oct 1, 2001
    • 0 Attachment
      Adam Judson wrote:
      >
      > --- In extremeprogramming@y..., Paul Michali <pcm@c...> wrote:
      > > Here's what I have now (still a work in progress, of course):
      > >
      >
      > Some thoughts.
      >
      > > public void checkAllPathSegments() {
      > > BasicReport primaryCurr = getLatestReportFor
      > (PRIMARY_PATH);
      > > BasicReport primaryPrev = getPreviousReportFor
      > (PRIMARY_PATH);
      >
      > I see this and I want to change it to :
      >
      > primaryReport.getNewData();
      > primaryReport.getPreviousData();
      >
      > I don't think I should be getting the data FOR the report object,
      > I think it should be gettting it itself.
      >
      > Hmmm, having said this, I don't really think this is what I want.
      > We probably need some sort of external agent to get the info
      > for us (HTML pages are the source no?)

      Yes HTML is the source. In the terminology that I use, I call this
      web page data a report, because in the domain, it is a report of
      road conditions.

      I do like what you have above. My version has a procedural flavor to
      it and I should probably change that.


      > >
      > > if ( primaryCurr.conditionStillBad(primaryPrev) ) {
      > >
      > > reportAnyChangeInPrimary(primaryCurr, primaryPrev);
      > > reportAnyChangeInSecondaries();
      > >
      >
      > - why am I passing data to an object to ask about the data? This data
      > and the logic feel like they belong together.

      There have been suggestions by others and it has made me think about
      having both a ScoutingReport object and a ReportHistory or some such
      object that knows about current and old conditions for a single path
      (and could tell me if it is still bad, better, worse, etc).


      > I hope I haven't misunderstood the problem to badly.

      Not too far off. Instead of there being one path that is composed of
      several sections, I have multiple, discrete paths, each with their
      own independent status and condition info.


      >
      > And having written this, I'm not sure how I'd get here from where
      > you are in small steps. So maybe this isn't helpfull after all.

      On the contrary. It gave me some other ideas to think about and is
      making me rethink about some of the objects I have and whether they
      are doing what they should be doing.

      Thanks for the reply!


      PCM (Paul Michali)

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

      Phone : (800) 572-6771 x 45817 (978) 244-5817 [direct]
      Paging: (800) 365-4578 [voice] pcm@... [email page]
    • Paul Michali
      ... Hmm. I was thinking about creating and passing the dispatcher object around (it know who to send the E-mail messages to, the name of the mail server, and
      Message 2 of 7 , Oct 1, 2001
      • 0 Attachment
        kevinxp@... wrote:
        >
        > --- In extremeprogramming@y..., Paul Michali <pcm@c...> wrote:
        > > Here's what I have now (still a work in progress, of course):
        > >
        > >
        > > public void makeObservations() {
        > > dispatcher.clearMessage();
        > > checkAllPathSegments();
        > > dispatcher.notifyObservers();
        > > }
        >
        > Would this be clearer if you passed a "message" object around? This message
        > would be cleared, populated, and then sent to observers.

        Hmm. I was thinking about creating and passing the dispatcher object
        around
        (it know who to send the E-mail messages to, the name of the mail
        server, and
        the message that needs to be sent). Now you have me thinking about
        whether
        I should pass a dispatcher or pass a Message object and then send it to
        the
        dispatcher object (of which the Message is just a "dumb" object that
        carries
        the message).



        > If a single object contained both primaryCurr and primaryPrev, you could ask
        > that object whether things were better, the same, or worse, with a simple
        > method (and no parameters). Same with secondary.

        Yeah. Some sort of ReportHistory object is starting to look better and
        better. I may give it a try today or tomorrow.


        >
        > What if this method was responsible for marking each report/segment/path as
        > "needs to report" or not. Then, a later method could sweep through and only
        > build a message from the ones that need to be included.

        I was thinking about this at one time, but I wasn't sure if it would pan
        out or not. I'll keep it in the back of my head.


        > > It does seem clearer (to me) so far. I'll keep plugging away at it.
        > > But, right now I've got to work on updating the test cases...
        >
        > I assume you mean _adding_ test cases. You wouldn't change the code in
        > ways that break tests, and then go back and fix the tests, would you?

        Nope. I created a set of parallel methods with new logic that does the
        same thing as the old. As a result, some of the new methods are not
        being tested, so I need to create some more test cases to test that
        logic.


        > Much
        > better to change the tests first, and then update the code to match.

        Yeah you're right (slapping my own hand). I ended up just extracting
        code to methods and naming them so that they made sense, joined some
        methods and then started looking at the tests to see how I could make
        sure I covered these "new" methods. I should work harder towards
        writing the test first and then coding to meet that test. With the
        refactoring, it's hard (for me) to see how to write the test before I
        do the refactoring step. I guess if I had a pair partner, it would
        really have helped here :^(

        I also didn't have great test coverage of the original code (a smell),
        because it did so much (like the original checkAllSegments() method
        where it collected a new report, compared them, built a message, and
        sent it out).

        You have some good ideas that I'm going to pursue, thanks!


        PCM (Paul Michali)

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

        Phone : (800) 572-6771 x 45817 (978) 244-5817 [direct]
        Paging: (800) 365-4578 [voice] pcm@... [email page]
      Your message has been successfully submitted and would be delivered to recipients shortly.