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

Re: [XP] Necessary comments?

Expand Messages
  • J. B. Rainsberger
    ... catch (BadLocationException ohWellWeDontCare) ... I changed your catch block. I hope you don t mind. :) This is similar to the how to write tests that
    Message 1 of 19 , Oct 1, 2003
    • 0 Attachment
      Brian Christopher Robinson wrote:

      > On Wed, 1 Oct 2003, Ron Jeffries wrote:
      >
      >
      >>So I would love to see a snippet of code from someone including a valuable
      >>comment saying WHY it should happen that way.
      >
      >
      > I wrote a simple program recently that allows the user to type in a
      > number and get that number printed out in decimal, hex, binary, and
      > ascii. In the code that handles the DocumentEvents of the JTextField
      > being edited, I have this:
      >
      > private void doUpdate(DocumentEvent event)
      > {
      > try
      > {
      > Document document = event.getDocument();
      > numberString_ = document.getText(0,
      > document.getLength());
      > updateText();
      > }
      catch (BadLocationException ohWellWeDontCare)
      > {
      > }
      > }

      I changed your catch block. I hope you don't mind. :)

      This is similar to the "how to write tests that catch exceptions"
      question in JUnit. Some prefer this:

      try {
      oops();
      fail("Didn't throw exception!");
      }
      catch (Exception e) {
      // Expected path
      }

      I do this:

      try {
      oops();
      fail("Didn't throw exception!");
      }
      catch (Exception expected) {}

      I think it communicates just as well.
      --
      J. B. Rainsberger,
      Diaspar Software Services
      http://www.diasparsoftware.com :: +1 416 791-8603
      Let's write software that people understand
    • Brian Christopher Robinson
      ... Yeah, that works too. I think the comment is a little easier to understand, since it draws your attention, but not by much. The reduced clutter might
      Message 2 of 19 , Oct 1, 2003
      • 0 Attachment
        On Wed, 1 Oct 2003, Ron Jeffries wrote:

        > catch (BadLocationException ignored) {}
        >
        > ? Both the parameter name and the spacing are intentional.

        Yeah, that works too. I think the comment is a little easier to
        understand, since it draws your attention, but not by much. The reduced
        clutter might make up for it.
      • Robert Blum
        Hi Ron! ... I ll give it a try. It s (for obvious reasons) not real production code, but it paraphrases the real code well enough. I hope. First, some
        Message 3 of 19 , Oct 1, 2003
        • 0 Attachment
          Hi Ron!

          >> Tests say what should happen. Code says how it should happen. But
          >> comments are invaluable for saying WHY it should happen that way.
          >

          >
          > So I would love to see a snippet of code from someone including a
          > valuable
          > comment saying WHY it should happen that way.

          I'll give it a try. It''s (for obvious reasons) not real production
          code, but it paraphrases the real code well enough. I hope.

          First, some background info: My target environment loves sending data
          using DMA. The DMA is driven by command tags and data. In our case, the
          data always follows the command tag. You can chain commands (and data)
          together by either putting them in sequence in memory, or you can issue
          a DMA command that actually calls another chain (which has to end in a
          return tag, or you will be punished :)

          Unfortunately, the return stack for those calls is small.

          The code in question is

          /* We could've used callChain instead, but we don't know
          * if there is any room in the DMA call stack left */
          masterChain.appendChain(someCommandChain);


          That comment is there since 'callChain' would be the more intuitive
          approach - usually we know if there's room left in the call stack. We
          just don't in this particular case.

          I can't check it, either - that would cost too much performance.

          Any suggestions how to express this better in code are appreciated.

          - Robert
        • Ilja Preuss
          ... Here are two suggestions to express this in code - wether it s actually *better* might be questionable: 1)
          Message 4 of 19 , Oct 2, 2003
          • 0 Attachment
            Robert Blum wrote:

            > First, some background info: My target environment loves sending data
            > using DMA. The DMA is driven by command tags and data. In our case,
            > the data always follows the command tag. You can chain commands (and
            > data) together by either putting them in sequence in memory, or you
            > can issue a DMA command that actually calls another chain (which has
            > to end in a return tag, or you will be punished :)
            >
            > Unfortunately, the return stack for those calls is small.
            >
            > The code in question is
            >
            > /* We could've used callChain instead, but we don't know
            > * if there is any room in the DMA call stack left */
            > masterChain.appendChain(someCommandChain);
            >
            >
            > That comment is there since 'callChain' would be the more intuitive
            > approach - usually we know if there's room left in the call stack. We
            > just don't in this particular case.
            >
            > I can't check it, either - that would cost too much performance.
            >
            > Any suggestions how to express this better in code are appreciated.

            Here are two suggestions to express this in code - wether it's actually
            *better* might be questionable:

            1) sendInPresenceOfUnknownRoomOnDmaStack(someCommandChain)

            2) write a unit test which simulates a full DMA call stack and therefore
            fails when you change the code to use callChain

            What do you think?

            Take care, Ilja
          • Dale Emery
            Hi Robert, ... Another thought: Could you implement callChain in a stack-friendly way? And another: If appendChain works, that must mean that it causes the
            Message 5 of 19 , Oct 2, 2003
            • 0 Attachment
              Hi Robert,

              > /* We could've used callChain instead, but we don't know
              > * if there is any room in the DMA call stack left */
              > masterChain.appendChain(someCommandChain);
              >
              > That comment is there since 'callChain' would be the more
              > intuitive approach - usually we know if there's room left in
              > the call stack. We just don't in this particular case.

              Another thought: Could you implement callChain in a
              stack-friendly way?

              And another: If appendChain works, that must mean that it causes
              the commands to be called without overflowing the stack. So
              could you force users to call appendChain instead of callChain?
              Or rename callChain with some private name, then rename
              appendChain as callChain? The underlying idea here is to move
              the stack-chewing code to a method that users can't call in
              limited-stack situations.

              Dale

              --
              Dale Emery -- Consultant -- Resistance as a Resource
              Web: http://www.dhemery.com
              Weblog: http://www.dhemery.com/journal (Conversations with Dale)
            • Robert Blum
              Hi Ilja! ... This one is slightly unwieldy :) More importantly though, I m not sending the chain - I m appending it to the call chain. Which involves copying
              Message 6 of 19 , Oct 2, 2003
              • 0 Attachment
                Hi Ilja!

                > Here are two suggestions to express this in code - wether it's actually
                > *better* might be questionable:
                >
                > 1) sendInPresenceOfUnknownRoomOnDmaStack(someCommandChain)

                This one is slightly unwieldy :) More importantly though, I'm not
                sending the chain - I'm appending it to the call chain. Which involves
                copying and hence is something the programmer should know of.
                (Performance requirements again)

                > 2) write a unit test which simulates a full DMA call stack and
                > therefore
                > fails when you change the code to use callChain

                Now this (Thank you to Dale, also) is great. I have no idea why I never
                thought of it - I'm already inspecting DMA chains I construct. I
                suspect the fact that the error "may or may not" occur blocked me from
                thinking about an environment where it *does* occur.

                I've got a couple of other code blocks that I'll tackle with that
                mindset today - we'll se what happens.

                - Robert
              • Robert Blum
                Hi Dale! ... I think there s a misunderstanding. The call stack lives on the DMA processor. The only stack space is the return address, but there s a (very
                Message 7 of 19 , Oct 2, 2003
                • 0 Attachment
                  Hi Dale!

                  > Another thought: Could you implement callChain in a
                  > stack-friendly way?

                  I think there's a misunderstanding. The call stack lives on the DMA
                  processor. The only stack space is the return address, but there's a
                  (very tight) limit on that.

                  > And another: If appendChain works, that must mean that it causes
                  > the commands to be called without overflowing the stack.

                  Yes - the chain in question is physically copied behind the last
                  command in the master chain.

                  > So
                  > could you force users to call appendChain instead of callChain?

                  Not really, because I don't know at runtime how deep the calls are
                  nested already. So, most of the time, callChain is the desired
                  function. I /want/ to use it in most of my code.

                  >
                  > Or rename callChain with some private name, then rename
                  > appendChain as callChain? The underlying idea here is to move
                  > the stack-chewing code to a method that users can't call in
                  > limited-stack situations.

                  The method stack space is not the issue - sorry I wasn't clear about
                  that.

                  - Robert
                • Ron Jeffries
                  ... First some things I m just wondering about: Does appendChain not have a performance hit over callChain? Or is that done in some other thread or something
                  Message 8 of 19 , Oct 2, 2003
                  • 0 Attachment
                    On Wednesday, October 1, 2003, at 10:34:17 PM, Robert Blum wrote:

                    > The code in question is

                    > /* We could've used callChain instead, but we don't know
                    > * if there is any room in the DMA call stack left */
                    > masterChain.appendChain(someCommandChain);


                    > That comment is there since 'callChain' would be the more intuitive
                    > approach - usually we know if there's room left in the call stack. We
                    > just don't in this particular case.

                    > I can't check it, either - that would cost too much performance.

                    First some things I'm just wondering about:

                    Does appendChain not have a performance hit over callChain? Or is that
                    done in some other thread or something that doesn't matter? I understand
                    that you need to scrape every cycle out, but would have thought that the
                    check wasn't very costly at all. Anyway, just wondering, not objecting.

                    For a fix, I'd consider renaming or aliasing appendChain (I suppose I
                    couldn't wrap it, that would cost an entire method call oh god) to
                    something like stackSizeUnknownChain(). Now /that/ method might need a
                    comment, but there'd be only one, and we programmers would quickly learn to
                    use this method when the stack size is ... unknown.

                    > Any suggestions how to express this better in code are appreciated.

                    Just musing ...

                    Ron Jeffries
                    www.XProgramming.com
                    You can observe a lot by watching. --Yogi Berra
                  • Robert Blum
                    Hi Ron! ... Yes, it does. It copies data around. But... ... Since it s copying data, I can use the DMA processor. (Told you the environment is DMA-happy!).
                    Message 9 of 19 , Oct 2, 2003
                    • 0 Attachment
                      Hi Ron!

                      > First some things I'm just wondering about:
                      >
                      > Does appendChain not have a performance hit over callChain?

                      Yes, it does. It copies data around. But...

                      > Or is that
                      > done in some other thread or something that doesn't matter?

                      Since it's copying data, I can use the DMA processor. (Told you the
                      environment is DMA-happy!). That means I'm not blowing out any cache
                      lines.

                      > I understand
                      > that you need to scrape every cycle out, but would have thought that
                      > the
                      > check wasn't very costly at all.

                      It wouldn't, if I had full control over the chain and could keep track
                      while it's created. Unfortunately, I don't - hence I would have to scan
                      the whole chain and follow all calls in there. That means it's quite
                      costly. (Especially following the call pointers. Cache misses cost an
                      arm and a leg)

                      > Anyway, just wondering, not objecting.
                      Definitely valid points - it is a strange environment.

                      > For a fix, I'd consider renaming or aliasing appendChain (I suppose I
                      > couldn't wrap it, that would cost an entire method call oh god)

                      You're going to kill me for this.... appendChain is inlined code.
                      Because, yes, a method call is hideously expensive. (Compared to other
                      architectures, at least)

                      > to
                      > something like stackSizeUnknownChain().

                      I like that. I'm tempted to roll it into a Macro (since the compiler
                      can't be trusted with inlining...), and that would be a nice name.

                      As a sidebar: Since I can't trust my compiler with inlining, and
                      there's a lot of nearly-duplicate code, I'm venturing deeper and deeper
                      into the lands of automated code generation. And I like what I'm seeing
                      - I can fully test-drive the generators, and any performance anxiety
                      for that code can be ignored.

                      I'm wondering if that is the path I've been looking for. I can be
                      cycle-counting-obsessed, and still write clear, decoupled, fully tested
                      code.

                      > Just musing ...

                      I appreciate you're taking the time.

                      - Robert
                    • Ron Jeffries
                      ... I m not troubled by inline code, if as you talk about below, it s generated by a macro or something so that I can edit it in one place and fix it in all,
                      Message 10 of 19 , Oct 2, 2003
                      • 0 Attachment
                        On Thursday, October 2, 2003, at 10:17:26 AM, Robert Blum wrote:

                        >> For a fix, I'd consider renaming or aliasing appendChain (I suppose I
                        >> couldn't wrap it, that would cost an entire method call oh god)

                        > You're going to kill me for this.... appendChain is inlined code.
                        > Because, yes, a method call is hideously expensive. (Compared to other
                        > architectures, at least)

                        I'm not troubled by inline code, if as you talk about below, it's generated
                        by a macro or something so that I can edit it in one place and fix it in
                        all, just like a method. We used to call those "open subroutines" I
                        believe.

                        >> to something like stackSizeUnknownChain().

                        > I like that. I'm tempted to roll it into a Macro (since the compiler
                        > can't be trusted with inlining...), and that would be a nice name.

                        > As a sidebar: Since I can't trust my compiler with inlining, and
                        > there's a lot of nearly-duplicate code, I'm venturing deeper and deeper
                        > into the lands of automated code generation. And I like what I'm seeing
                        > - I can fully test-drive the generators, and any performance anxiety
                        > for that code can be ignored.

                        Yes, it can be a good thing to do. Sometimes I've gone too deep into the
                        bag of tricks in doing things like that. Most of the payoff probably comes
                        early.

                        > I'm wondering if that is the path I've been looking for. I can be
                        > cycle-counting-obsessed, and still write clear, decoupled, fully tested
                        > code.

                        Yep. Could be just the thing.

                        >> Just musing ...

                        > I appreciate you're taking the time.

                        It's more fun than what I /should/ be doing ... ;->

                        Ron Jeffries
                        www.XProgramming.com
                        One test is worth a thousand expert opinions.
                        -- Bill Nye (The Science Guy)
                      Your message has been successfully submitted and would be delivered to recipients shortly.