Skip to content
  • Categories
  • Recent
  • Tags
  • Popular
  • World
  • Users
  • Groups
Skins
  • Light
  • Cerulean
  • Cosmo
  • Flatly
  • Journal
  • Litera
  • Lumen
  • Lux
  • Materia
  • Minty
  • Morph
  • Pulse
  • Sandstone
  • Simplex
  • Sketchy
  • Spacelab
  • United
  • Yeti
  • Zephyr
  • Dark
  • Cyborg
  • Darkly
  • Quartz
  • Slate
  • Solar
  • Superhero
  • Vapor

  • Default (No Skin)
  • No Skin
Collapse
Code Project
  1. Home
  2. The Lounge
  3. Code Review

Code Review

Scheduled Pinned Locked Moved The Lounge
code-review
11 Posts 7 Posters 0 Views 1 Watching
  • Oldest to Newest
  • Newest to Oldest
  • Most Votes
Reply
  • Reply as topic
Log in to reply
This topic has been deleted. Only users with topic management privileges can see it.
  • A Abhijit Ghosh Subho

    "CODE REVIEW" - This is a phrase that my boss uses everyday. I am not sure what it exactly means. So everytime I make a work log, I put in "Code Review" against my spare time. Now I am 110% efficient and he likes that.

    R Offline
    R Offline
    Ravi Bhavnani
    wrote on last edited by
    #2

    Here's a Wikipedia[^] definition. Where I work, code reviews are done for every check-in (sometimes after the fact).  Although the devs I work with are a bright and seasoned bunch, that doesn't exclude anyone from a code review.  Our code reviews mainly focus on performance and security and serve as a "second pair of eyes" to help catch the odd bug.  Reviewers rarely end up suggesting rewriting comments or renaming identifiers, because we have a set of guidelines already in place that addresses that sort of stuff.  Our code reviews usually take no more than 10-15 minutes and are almost always limited to the developer and a single reviewer. I've learned a lot from having my own code reviewed. /ravi

    My new year resolution: 2048 x 1536 Home | Articles | My .NET bits | Freeware ravib(at)ravib(dot)com

    Kornfeld Eliyahu PeterK 1 Reply Last reply
    0
    • R Ravi Bhavnani

      Here's a Wikipedia[^] definition. Where I work, code reviews are done for every check-in (sometimes after the fact).  Although the devs I work with are a bright and seasoned bunch, that doesn't exclude anyone from a code review.  Our code reviews mainly focus on performance and security and serve as a "second pair of eyes" to help catch the odd bug.  Reviewers rarely end up suggesting rewriting comments or renaming identifiers, because we have a set of guidelines already in place that addresses that sort of stuff.  Our code reviews usually take no more than 10-15 minutes and are almost always limited to the developer and a single reviewer. I've learned a lot from having my own code reviewed. /ravi

      My new year resolution: 2048 x 1536 Home | Articles | My .NET bits | Freeware ravib(at)ravib(dot)com

      Kornfeld Eliyahu PeterK Offline
      Kornfeld Eliyahu PeterK Offline
      Kornfeld Eliyahu Peter
      wrote on last edited by
      #3

      Once we tried to do code review, but because of the nature of the code we have to gave up - it's mostly write-only code... :(

      I'm not questioning your powers of observation; I'm merely remarking upon the paradox of asking a masked man who he is. (V)

      "It never ceases to amaze me that a spacecraft launched in 1977 can be fixed remotely from Earth." ― Brian Cox

      OriginalGriffO B 2 Replies Last reply
      0
      • Kornfeld Eliyahu PeterK Kornfeld Eliyahu Peter

        Once we tried to do code review, but because of the nature of the code we have to gave up - it's mostly write-only code... :(

        I'm not questioning your powers of observation; I'm merely remarking upon the paradox of asking a masked man who he is. (V)

        OriginalGriffO Offline
        OriginalGriffO Offline
        OriginalGriff
        wrote on last edited by
        #4

        Brrrr! :shudder:

        Those who fail to learn history are doomed to repeat it. --- George Santayana (December 16, 1863 – September 26, 1952) Those who fail to clear history are doomed to explain it. --- OriginalGriff (February 24, 1959 – ∞)

        "I have no idea what I did, but I'm taking full credit for it." - ThisOldTony
        "Common sense is so rare these days, it should be classified as a super power" - Random T-shirt

        Kornfeld Eliyahu PeterK 1 Reply Last reply
        0
        • OriginalGriffO OriginalGriff

          Brrrr! :shudder:

          Those who fail to learn history are doomed to repeat it. --- George Santayana (December 16, 1863 – September 26, 1952) Those who fail to clear history are doomed to explain it. --- OriginalGriff (February 24, 1959 – ∞)

          Kornfeld Eliyahu PeterK Offline
          Kornfeld Eliyahu PeterK Offline
          Kornfeld Eliyahu Peter
          wrote on last edited by
          #5

          Tell me about that... I live it every day... All of our code base is in C# now (it's over 5 million lines of code), but when reading parts of it I still can smell COBOL from 20 years before...

          I'm not questioning your powers of observation; I'm merely remarking upon the paradox of asking a masked man who he is. (V)

          "It never ceases to amaze me that a spacecraft launched in 1977 can be fixed remotely from Earth." ― Brian Cox

          L 1 Reply Last reply
          0
          • Kornfeld Eliyahu PeterK Kornfeld Eliyahu Peter

            Tell me about that... I live it every day... All of our code base is in C# now (it's over 5 million lines of code), but when reading parts of it I still can smell COBOL from 20 years before...

            I'm not questioning your powers of observation; I'm merely remarking upon the paradox of asking a masked man who he is. (V)

            L Offline
            L Offline
            Lost User
            wrote on last edited by
            #6

            Could you show an example of that?

            Kornfeld Eliyahu PeterK 1 Reply Last reply
            0
            • L Lost User

              Could you show an example of that?

              Kornfeld Eliyahu PeterK Offline
              Kornfeld Eliyahu PeterK Offline
              Kornfeld Eliyahu Peter
              wrote on last edited by
              #7

              Do you know COBOL's STRING statement?

              string Msg = FName + " " + LName + ", " + City;

              is almost identical to

              STRING FNAME SPACE LNAME ", " CITY DELIMITED BY SIZE INTO MSG.

              We also have a lot of code using invalid value initialization for variables, like

              int Msl = -1;

              While in new code we use null... In COBOL we used LOW-VALUE as no NULL was available...

              I'm not questioning your powers of observation; I'm merely remarking upon the paradox of asking a masked man who he is. (V)

              "It never ceases to amaze me that a spacecraft launched in 1977 can be fixed remotely from Earth." ― Brian Cox

              A 1 Reply Last reply
              0
              • Kornfeld Eliyahu PeterK Kornfeld Eliyahu Peter

                Do you know COBOL's STRING statement?

                string Msg = FName + " " + LName + ", " + City;

                is almost identical to

                STRING FNAME SPACE LNAME ", " CITY DELIMITED BY SIZE INTO MSG.

                We also have a lot of code using invalid value initialization for variables, like

                int Msl = -1;

                While in new code we use null... In COBOL we used LOW-VALUE as no NULL was available...

                I'm not questioning your powers of observation; I'm merely remarking upon the paradox of asking a masked man who he is. (V)

                A Offline
                A Offline
                Abhijit Ghosh Subho
                wrote on last edited by
                #8

                The code I have been reviewing since the past few days is a VBA application. The developers used a lot of xml files to store all the conditional statements and dynamic data. The VB code contains subs like Call_ARF_something_something_something. I mean the subs are sctually named like that. So if there is call statement, it looks like Call Call_ARF_something. Apart from that, based on actual selections,new xml nodes are formed(by concateneting underscore and selections) and then the dynamic data is recorded inside those nodes. I found nodes like XML_Node_ARF_PFG_PFGOption_CFG_CFGOption_(......... and so on) It's a nightmare for me since there is no documentation and I have to figure out the business logic from all of this.

                R 1 Reply Last reply
                0
                • A Abhijit Ghosh Subho

                  The code I have been reviewing since the past few days is a VBA application. The developers used a lot of xml files to store all the conditional statements and dynamic data. The VB code contains subs like Call_ARF_something_something_something. I mean the subs are sctually named like that. So if there is call statement, it looks like Call Call_ARF_something. Apart from that, based on actual selections,new xml nodes are formed(by concateneting underscore and selections) and then the dynamic data is recorded inside those nodes. I found nodes like XML_Node_ARF_PFG_PFGOption_CFG_CFGOption_(......... and so on) It's a nightmare for me since there is no documentation and I have to figure out the business logic from all of this.

                  R Offline
                  R Offline
                  Ravi Bhavnani
                  wrote on last edited by
                  #9

                  Abhijit Ghosh (Subho) wrote:

                  there is no documentation

                  Set pet peeve #1 here[^]. :( /ravi

                  My new year resolution: 2048 x 1536 Home | Articles | My .NET bits | Freeware ravib(at)ravib(dot)com

                  1 Reply Last reply
                  0
                  • Kornfeld Eliyahu PeterK Kornfeld Eliyahu Peter

                    Once we tried to do code review, but because of the nature of the code we have to gave up - it's mostly write-only code... :(

                    I'm not questioning your powers of observation; I'm merely remarking upon the paradox of asking a masked man who he is. (V)

                    B Offline
                    B Offline
                    Brisingr Aerowing
                    wrote on last edited by
                    #10

                    At least it isn't APL! On a side note, I know someone who can READ APL code! No joke, he even demonstrated it with random code someone who was visiting from Germany came up with!

                    What do you get when you cross a joke with a rhetorical question?

                    B 1 Reply Last reply
                    0
                    • B Brisingr Aerowing

                      At least it isn't APL! On a side note, I know someone who can READ APL code! No joke, he even demonstrated it with random code someone who was visiting from Germany came up with!

                      What do you get when you cross a joke with a rhetorical question?

                      B Offline
                      B Offline
                      BobJanova
                      wrote on last edited by
                      #11

                      APL is not that bad if you know it. That's like saying Hungarian is a bad language because not many people can understand it.

                      1 Reply Last reply
                      0
                      Reply
                      • Reply as topic
                      Log in to reply
                      • Oldest to Newest
                      • Newest to Oldest
                      • Most Votes


                      • Login

                      • Don't have an account? Register

                      • Login or register to search.
                      • First post
                        Last post
                      0
                      • Categories
                      • Recent
                      • Tags
                      • Popular
                      • World
                      • Users
                      • Groups