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

Re: [XP] Necessary comments?

Expand Messages
  • Ken Boucher
    ... We seem to be talking about two different things here: 1) Comments added by the programmer for clairity. 2) Comments mandated by the organization.
    Message 1 of 19 , Oct 1, 2003
    • 0 Attachment
      > In OO programming a class responsibility comment is normally the
      > most that is needed. Method names and parameters should then speak
      > for themselves. I've seen companies that mandate big fill in the
      > form style of comments on top of every method. They are worse than
      > useless while they hide the code. At least witht he current IDE's
      > the comments font and color can be set to fade away next to the
      > actual code.

      We seem to be talking about two different things here:
      1) Comments added by the programmer for clairity.
      2) Comments mandated by the organization.

      Unfortunately, #2 seems to be consistantly used as a weapon to beat
      up #1. I've seen it in this post, in posts about comment strippers,
      and in a few other places.

      A comment is a tool. If it's the only tool a programmer or an
      organization has, it's not going to use it well. If someone doesn't
      have the understanding to choose good method names or to even put the
      method in the right class, then yes, comments will be used poorly.

      On the other hand, while great method names, wonderful classes,
      metaphors, and brilliant design lessen the need for comments, they
      are no reason to argue that a perfectly serviceable tool should not
      be used when appropriate.

      Tests say what should happen. Code says how it should happen. But
      comments are invaluable for saying WHY it should happen that way.
    • Ron Jeffries
      ... This sounds very true. Still, in all my years, I have seen very few comments that I actually appreciated. (Of course, many of my own comments here are not
      Message 2 of 19 , Oct 1, 2003
      • 0 Attachment
        On Wednesday, October 1, 2003, at 6:46:11 AM, Ken Boucher wrote:

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

        This sounds very true. Still, in all my years, I have seen very few
        comments that I actually appreciated. (Of course, many of my own comments
        here are not as appreciated as they might be, so maybe it all averages
        out.)

        Plus, I remember wondering once recently (I believe it was in about 1999)
        why some particular code was the way it was. (The preceding is not sarcasm.
        I recall a day on C3 when we were all wondering why something had been done
        the way it was.) We could have used a comment on that day.

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

        Then, of course, we'll try to refactor it to see how much of the comment
        can be reflected in the code, and how much cannot.

        Ron Jeffries
        www.XProgramming.com
        I could be wrong, of course. It's just not the way to bet.
      • Brian Christopher Robinson
        ... 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
        Message 3 of 19 , Oct 1, 2003
        • 0 Attachment
          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 e)
          {
          // oh well we don't care
          }
          }

          I often have empty catches like this where I really don't care about the
          exception but Java forces me to catch it. In this case, the exception
          shouldn't occur since the getText() call is safe. Even if it did, doing
          nothing would be the appropriate action.
        • Ron Jeffries
          ... What about catch (BadLocationException ignored) {} ? Both the parameter name and the spacing are intentional. Ron Jeffries www.XProgramming.com Sorry about
          Message 4 of 19 , Oct 1, 2003
          • 0 Attachment
            On Wednesday, October 1, 2003, at 1:11:15 PM, 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 e)
            > {
            > // oh well we don't care
            > }
            > }

            > I often have empty catches like this where I really don't care about the
            > exception but Java forces me to catch it. In this case, the exception
            > shouldn't occur since the getText() call is safe. Even if it did, doing
            > nothing would be the appropriate action.

            What about

            catch (BadLocationException ignored) {}

            ? Both the parameter name and the spacing are intentional.

            Ron Jeffries
            www.XProgramming.com
            Sorry about your cow ... I didn't know she was sacred.
          • 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 5 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 6 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 7 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 8 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 9 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 10 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 11 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 12 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 13 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 14 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.