Beefy Boxes and Bandwidth Generously Provided by pair Networks
Think about Loose Coupling
 
PerlMonks  

Code Review section, anyone?

by merlyn (Sage)
on Sep 14, 2000 at 04:19 UTC ( #32404=monkdiscuss: print w/ replies, xml ) Need Help??

In RE: RE: Re: Perl secrets, I muse:
It would be nice if someone at work coded some, specifically Perl, ya' know, someone to peruse my code on occassion, but there's nobody.
But think of the experts you have here! Maybe I need to get vroom to create a section called Code Review, where we could post snippets (perhaps anonymously, with a private feedback link) and let others issue code review, but telling still others that this is perhaps questionable code.
Hmm. An interesting idea. A place for monks to get honest, open feedback about their code, flagged in a sense that this "might not be the best way to do it", or even "this may be a pretty narrow application". Nobody posts here unless they want direct feedback.

Any takers? Any interest? Can the everything code already handle some sort of private feedback, or would it all have to be public nodes?

-- Randal L. Schwartz, Perl hacker

Edit: 2001-03-03 by neshura

Comment on Code Review section, anyone?
RE: Code Review section, anyone?
by mirod (Canon) on Sep 14, 2000 at 04:32 UTC

    Reviewing the code in modules would be a start, as it is likely to be re-used by unsuspecting brothers

    As modules are usually too big to be reviewed completely maybe we could define standard comments that could be used by author to indicate that a portion of code, or a function should be reviewed.

    Something like:

    #HU-HO (review anyone?) $;=$";
RE: Code Review section, anyone?
by the_slycer (Chaplain) on Sep 14, 2000 at 11:22 UTC
    I personally would love to see this come about. I am in the
    same boat as BastardOperator in that I am working my way
    through the books, and writing apps as I go. No previous
    coding experience to speak of, and no-one at work to take
    a look at my code.
    I would post the odd snippet to a node like that
RE: Code Review section, anyone?
by Jouke (Curate) on Sep 14, 2000 at 11:32 UTC
    Sure, good idea, but I think it should be 'small pieces of code' and not 10 pages of code to review. I think no one has the time to review such large scripts.

    But I would certainly love the possibility of posting a subroutine to be reviewed because I was not certain if it was the most efficient/fastest/cleanest way to do it, for example

    Jouke Visser, Perl 'Adept'
RE: Code Review section, anyone?
by little (Curate) on Sep 14, 2000 at 14:36 UTC
    I think it's a great and usefull idea, which I'd be pleased to use as well.
    In the past days I've seen a lot of code that made me saying : "Oh why the heck did I do this all the way around?"
    And I agree, sometimes a risk for the satability of the Prog or even the entire network arises from two lines of code, but there's still the possibility to send a link on larger pieces of code to the "helping wizard".
    In fact, I was suprised to hear merlyn say "... if you need a review of your code, so just send it to me". I wouldn't like to bother the perl guru's with my problem's while I'm afraid that those occure due to my nonsatisfying knowledge. Yes, I'd be afraid of blaming myself truly.
    b.t.w. Folks, you are great! :-)
RE (tilly) 1: Code Review section, anyone?
by tilly (Archbishop) on Sep 14, 2000 at 15:18 UTC
    I think it is an interesting idea, but would work better in theory than practice. Here are my main concerns:
    1. I am more likely to take out energy to answer a more obscure question if others have not answered it. With this system I would tend to assume that it was answered and wouldn't answer. I suspect others would do likewise.
    2. Conversely if the question is basic, there are a lot of people who will want to take the chance to answer one that they can get, and the hapless questioner would get a flood of answers from people who could not see others.
    3. I think that some of the most valuable answers given come as feedback, improvements, and corrections on answers others gave. This would remove opportunities for that.
    4. For very short questions it is already fine to use /tell in the chatterbox, edit your home node, etc. People already do this.
RE: Code Review section, anyone?
by BigJoe (Curate) on Sep 14, 2000 at 17:51 UTC
    I think this would be a great place for us to learn. I know myself I have a real narrow use of Perl currently and I would like to change that. I don't have any projects that are requiring me to learn and use more than what I already know. This would be a great oppertunity for alot of us that don't currently use this on a broad scale to be able to get some real world experience doing it.

    --BigJoe

    Learn patience, you must.
    Young PerlMonk, craves Not these things.
    Use the source Luke.
RE: Code Review section, anyone?
by jreades (Friar) on Sep 14, 2000 at 18:21 UTC

    In no particular order....

    • Isn't code review offered in principle by both Seekers of Perl Wisdom and, for shorter questions, Q & A?
    • At the level of the slycer and BastardOperator and, I hope, myself you run into a thorny problem -- most, although by no means all, of my mistakes will be structural rather than procedural (unless I'm venturing into uncharted territory -- in which case I'm probably back at the level of quick and dirty answers).
    • This makes it extremely difficult for someone to review a section of my code and say: "this could be done better," because doing so would involve a review of all the code... which is generally going to be well over 300 lines
    • Speaking for myself, I also come from a non-traditional programming background (uh, Comparative Literature) and have reached the point where I need to understand more about the why before I can move forward again with the how. I've been trying to work my way through Mastering Algorithms (and getting bogged down in b-trees) but it makes me think that a section devoted to strategy rather than review might be more helpful.
    • After all, most of the time I waste is going back when I discover that a particularl methodology doesn't get me where I need to be, not that a particular subroutine in causing me problems.

    I hope this makes sense, because I'm arguing not for a section where code is excluded but where the discussion 'decomposes' into code from a meta-programmatic level -- e.g. I'm writing an application to watch a log-file and report on irregular behavior -- I know that I need to mask the process itself so that a hacker won't realize they're being observed, and I also need a way to recover any data they might have changed. Where should I look for more information about this? And what are the common traps that people writing this kind of program fall into?

    The thing that I like about this idea is that it enables me to ask complex questions and get useful answers without either a) expecting anyone to give up a large amount of their time reviewing the implementation, or b) being handed the answer on a plate which doesn't help me learn.

    Hope this makes sense.

      I recommend that for this kind of problem you start with some general programming books (eg "Code Complete" and "The Pragmatic Programmer") and go from there. Overall, "How do I structure what I am doing" questions are very important but far too seldom considered.

      I mean to post some day on a few of these issues. Thinking in terms of transactions. Modularization and loose coupling. etc. However so far the majority of that discussion around here stops at, "Avoid Cargo Cult programming and don't bother reinventing wheels."

      But the next time you are starting to write something, why not spec out the general flow, and post that asking for general comments? The question may have no Perl per se in it, but the answers should be helpful and relevant to Perl programmers...

RE: Code Review section, anyone?
by swiftone (Curate) on Sep 14, 2000 at 19:25 UTC
    Posting problematic code for review is already done in Seekers of Perl Wisdom, so the key issue appears to be the private feedback.

    I don't really understand the point. Why get private feedback when the feedback can be posted publicly, to help others? There doesn't even have to be a experience issue, because I (and I'm assumin others) tend to upvote questions that illicit good responses.

    A place for monks to get honest, open feedback about their code,....Nobody posts here unless they want direct feedback.

    This sounds like PerlMonks :) You can even post Anonymously if you like.

      A place for monks to get honest, open feedback about their code,....Nobody posts here unless they want direct feedback.
      This sounds like PerlMonks :) You can even post Anonymously if you like.
      No, the problem is that many monks post to Code or Snippets without there being the request (and understanding) that the code will get feedback. I've stomped on far too many toes here, because I dig in with my "code review" eyes on, when all they wanted to do was say "see, looky here, I got some GOOD STUFF".

      So, the difference between "code review" and the rest is that there is an explicit understanding that this is the "I want feedback on how I am doing this". Formalization of the process would definitely help, and yet still protect the ego-full junior programmers (opposite of egoless programming) from misunderstanding the responses of people like me, who ultimately are just trying to help.

      -- Randal L. Schwartz, Perl hacker

        Okay, let me see if I understand completely:

        You want a section that is different from Seekers of Perl Wisdom in that:

        1. It is explicitly for reviewing working code
        2. The posters are aware that the code is there to be reviewed
        3. Replies can be private
        If this is indeed what you want, I would overall have no problem with it, except:
        1. I see no reason for private feedback. PerlMonks is about sharing learning, and this could be an excellent learning opportunity for more than the poster
        2. This would be best accomplished by revising the Code Catacombs section. As it stands, there is no real distinction made between Code Catacombs, Craft, and Snippets. Rather than adding a new section, I would modify one of those existing ones.
        If I am now understanding your wishes, and with the above comments, I think this is a great idea. You mentioned something in the chatterbox yesterday about what you considered good/bad usage of the ? : construction during a code-review, and I found it interesting. Where I work I'm the only Perl programmer, and there is nothing resembling a code review. My only reason for writing good code is that I have to maintain it, and my only means of learning new tricks is reading books and PerlMonks. I know I would benefit from code review, and I imagine others would too.

        All of this is IMNSHO, of course.

      I think Seekers is quite different from what was proposed for the code review section.

      Seekers:
      the code doesn't work; the poster is stuck. Post the minimal example and describe errors.
      Code Review:
      the code works, but the poster wants to know if there's a way to improve a function's efficiency, or if there are some bugs in the module that s/he hasn't seen, or if there's a pitfall in the program s/he didn't notice.

      That said, I think Review is more or less like Craft.

      See the description of the various sections. I get the impression that snippets and catacombs could easily be used for polished pieces of (snippets and standalone programs respectively) code (polished modules belong at CPAN of course). The code in these sections should be good enough for someone to use. It might actually be nice to have these sections moderated, allowing only code that passed review to be found in these sections.

      Craft, on the other hand, is a place to show off code. If something's on display, it's going to get comments and critique...at least that's how I see it.

      Update: Merlyn, I agree that it's good to have it clear that people will critique. I just think a new section would be redundant. If Craft is what I think it is (and if not, what is it for?), it would certainly benefit from a clearer description, and possibly even from being renamed to Review.

        Craft, on the other hand, is a place to show off code. If something's on display, it's going to get comments and critique...at least that's how I see it.
        But you perhaps are a mature programmer, someone who understands that putting stuff out there is going to get critique. I'm worried about a repeat of a couple of mistakes I made in the past here, where someone is posting a snippet "because it's way cool, dude" and not for critique, and then I critique it.

        I want a place where it's clear from the get go that code will get shredded in that area. So the newbies with fragile egos will stay away, but the people who understand what having a good code critique will treasure.

        -- Randal L. Schwartz, Perl hacker

        I think that Code Catacombs is already moderated in some manner.   I've submitted a small handful of minor scripts, one of which actually appears there.   Mind you, I'm not complaining - just hope to learn enough at PM to eventually tweak or rewrite the others to warrant inclusion.   8^)
            cheers,
            Don
            striving for Perl Adept
RE: Code Review section, anyone?
by ivory (Pilgrim) on Sep 15, 2000 at 03:45 UTC
    I like this idea. I think it would be very helpful to have a place to get feedback, but I'd like to see this section help newbies as well as the more experienced programmers.

    Let me know what you all this of this: why not have a "learning project of the week/month"? Basically, we would give the specifications of a programming project (accessable to less experianced monks, but not a "hello world" type program) and there could be a section devoted to these projects. All monks who feel that they would benefit from completing the project could do it, and then post their code for review. More experianced monks could help by posting their own solutions (shouldn't take too long) and explaining why they did it the way they did, or by giving feedback on the other solutions posted.

    The biggest problem I face, personally, when learning something new is not actually getting the practice and feedback I need. I am guessing that a lot of newer monks are in the same boat and would like to take on learning projects and then get feedback on them. There ought to be an understandig from the start that the solutions to these projects would be submitted by monks who are tryng their best and want to learn...thus those giving feedback should take care to give constructive criticism, rather than blasting them for not seeing the "obviously better" solution.

    The projects should be fun and interesting. I'm sure a lot of you have projects that you've thought of or done that would be suitable.

    ivory

RE: Code Review section, anyone?
by doon (Initiate) on Sep 15, 2000 at 17:20 UTC
    I think that's a very good idea. It would serve, I think, to improve peoples code, as long as the review comments where constructive.
RE: Code Review section, anyone?
by jepri (Parson) on Sep 20, 2000 at 05:17 UTC

    I love the idea. You could even extend it a little further and call it the For Review Section. I've just been doing a bit of IPC and I was thinking of writing something up and posting it to tutorials. With a reviews section I could let people tear it apart and then post the revised version. That way I wouldn't have to worry about posting mistakes for others to follow.

    _______________
    Jeremy

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: monkdiscuss [id://32404]
Approved by root
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others meditating upon the Monastery: (11)
As of 2014-08-22 12:02 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    The best computer themed movie is:











    Results (156 votes), past polls