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. Reviewing code...

Reviewing code...

Scheduled Pinned Locked Moved The Lounge
collaborationquestionlearning
28 Posts 12 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.
  • H Hosey

    Cheers for the pointer. Was only posted in the lounge as I was sharing general chit-chat we were having about it in the office. Will post correctly in future :)

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

    Don't worry about it, it appears to have generated a lot of traffic so that would suggest it is perfectly adequete post for the lounge. My mistake.

    H 1 Reply Last reply
    0
    • L Lost User

      Don't worry about it, it appears to have generated a lot of traffic so that would suggest it is perfectly adequete post for the lounge. My mistake.

      H Offline
      H Offline
      Hosey
      wrote on last edited by
      #13

      :thumbsup: :cool: all is well :)

      1 Reply Last reply
      0
      • N NormDroid

        I smell danger, I smell VB.

        Two heads are better than one.

        H Offline
        H Offline
        Hosey
        wrote on last edited by
        #14

        An old-skool legacy im trying to get rectified. But its a large code-base, and wont get changed overnight :) Personally use C# in all my "pet-projects" but have to stick to VB at the office. Worth sharing, just to make people cringe, but we still have a business critical application (used daily and directly linked to our revenues) that was written in VB 6 over 12 yrs ago... still going... Dont have the time to re-write it at the moment, and if it aint broke, dont fix it... :)

        N 1 Reply Last reply
        0
        • D Dalek Dave

          I think it is sensible, you never know when the value of Zero changes! :)

          ------------------------------------ I will never again mention that I was the poster of the One Millionth Lounge Post, nor that it was complete drivel. Dalek Dave

          H Offline
          H Offline
          Hosey
          wrote on last edited by
          #15

          HAHAH wasnt thinking outside the box like that... good man! ;P :-D

          1 Reply Last reply
          0
          • H Hosey

            An old-skool legacy im trying to get rectified. But its a large code-base, and wont get changed overnight :) Personally use C# in all my "pet-projects" but have to stick to VB at the office. Worth sharing, just to make people cringe, but we still have a business critical application (used daily and directly linked to our revenues) that was written in VB 6 over 12 yrs ago... still going... Dont have the time to re-write it at the moment, and if it aint broke, dont fix it... :)

            N Offline
            N Offline
            NormDroid
            wrote on last edited by
            #16

            Hosey^ wrote:

            and if it aint broke, dont fix it...

            VB code is always broke ;)

            Two heads are better than one.

            1 Reply Last reply
            0
            • H Hosey

              I cannot deny that I have been guilty of such code sins, sometimes I write less than efficient code as I fire through a problem, but I dont think I've ever written redundant code like the example I found. Set a variable to a zero-length string, then on the very next line check it is a zero-length string... hahaha

              P Offline
              P Offline
              Pualee
              wrote on last edited by
              #17

              If the code was C and there were poorly managed global variables you could not otherwise control... I can understand why.

              1 Reply Last reply
              0
              • H Hosey

                I often find myself having to review code within all of our systems as our company grows/changes, as im sure most of us have to during the course of work. Sometimes I find myself looking at old code, and feeling embaressed for the coder (who is sometimes myself) as code that was applicable due to knowledge at the time looks frightful in the light of newer knowledge and skills, but occasionally I come across absolute gems. The coder who wrote this (who shall remain unamed) is no longer with the company, but I do not regret the loss one bit in the face of massive amounts of code being uncovered of this stunning calibre; Classic ASP:

                If Request.ServerVariables("REQUEST_METHOD") = "POST" Then
                If Request.Form("btnAddTask") <> "" Then
                For Each obj In Request.Form

                iSectionID_New = ""
                If Len(iSectionID_New) = 0 Then

                iSectionID_New = Trim(Request.Form("section_id_filter"))
                End If

                iTaskID_New = ""
                If Len(iTaskID_New) = 0 Then

                iTaskID_New = Trim(Request.Form("task_new"))
                End If

                iDisplayOrder_New = ""
                If Len(iDisplayOrder_New) = 0 Then

                If Len(Trim(Request.Form("display_order_new"))) > 0 And IsNumeric(Trim(Request.Form("display_order_new"))) Then
                iDisplayOrder_New = Trim(Request.Form("display_order_new"))
                End If
                End If

                iActionOrder_New = ""
                If Len(iActionOrder_New) = 0 Then

                If Len(Trim(Request.Form("action_order_new"))) > 0 And IsNumeric(Trim(Request.Form("action_order_new"))) Then
                iActionOrder_New = Trim(Request.Form("action_order_new"))
                End If
                End If
                Next
                End if
                End if

                the bold sections are the ones that immediately drew the attention of the team, but we then looked at the logic itself, and were stunned by the loop itself For Each obj In Request.Form which not only is never used, but just increases the number of iterations of the same code for absolutely no reason whatsoever... Anyone else have the joy of such discoveries?? Any clangers found that left you slack-jawed and slightly disoriented?? :) :)

                A Offline
                A Offline
                Amar Chaudhary
                wrote on last edited by
                #18

                Have fun :)

                List<List<urlKeeper>> PhraseResultSet = new List<List<urlKeeper>>();
                c.Urls = u.AltavistaScrapper(c.Url);
                for (int i = 0; i < uniqueKeywords.Count; i++)
                {
                PhraseResultSet.Add(u.AltavistaPhraseScrapper(uniqueKeywords[i]));

                        // get ranking for comps 
                        c.Competitors.FindAll(delegate(Competitors x)
                        {
                            return uniqueKeywords\[i\] == x.Phrase;
                        }).ForEach(delegate(Competitors x)
                        {
                            PhraseResultSet\[i\].Sort(new Comparison<urlKeeper>(delegate(urlKeeper obj1, urlKeeper obj2)
                            {
                                return obj1.Rank.CompareTo(obj2.Rank);
                            }));
                
                            int index = PhraseResultSet\[i\].FindIndex(delegate(urlKeeper y)
                            {
                                return (y.HostName.ToUpper().Replace("HTTP://", "")
                                                            .Replace("HTTPS://", "")
                                                            .Replace("WWW.", "")
                                    == x.Url.ToUpper().Replace("HTTP://", "")
                                                      .Replace("HTTPS://", "")
                                                      .Replace("WWW.", ""));
                            });
                            if (index >= 0)
                                x.Rank = PhraseResultSet\[i\]\[index\].Rank;
                            else
                                x.Rank = -1;
                
                        }); ;
                    }
                

                My Startup!!!!
                Profile@Elance - feedback available too

                1 Reply Last reply
                0
                • H Hosey

                  I often find myself having to review code within all of our systems as our company grows/changes, as im sure most of us have to during the course of work. Sometimes I find myself looking at old code, and feeling embaressed for the coder (who is sometimes myself) as code that was applicable due to knowledge at the time looks frightful in the light of newer knowledge and skills, but occasionally I come across absolute gems. The coder who wrote this (who shall remain unamed) is no longer with the company, but I do not regret the loss one bit in the face of massive amounts of code being uncovered of this stunning calibre; Classic ASP:

                  If Request.ServerVariables("REQUEST_METHOD") = "POST" Then
                  If Request.Form("btnAddTask") <> "" Then
                  For Each obj In Request.Form

                  iSectionID_New = ""
                  If Len(iSectionID_New) = 0 Then

                  iSectionID_New = Trim(Request.Form("section_id_filter"))
                  End If

                  iTaskID_New = ""
                  If Len(iTaskID_New) = 0 Then

                  iTaskID_New = Trim(Request.Form("task_new"))
                  End If

                  iDisplayOrder_New = ""
                  If Len(iDisplayOrder_New) = 0 Then

                  If Len(Trim(Request.Form("display_order_new"))) > 0 And IsNumeric(Trim(Request.Form("display_order_new"))) Then
                  iDisplayOrder_New = Trim(Request.Form("display_order_new"))
                  End If
                  End If

                  iActionOrder_New = ""
                  If Len(iActionOrder_New) = 0 Then

                  If Len(Trim(Request.Form("action_order_new"))) > 0 And IsNumeric(Trim(Request.Form("action_order_new"))) Then
                  iActionOrder_New = Trim(Request.Form("action_order_new"))
                  End If
                  End If
                  Next
                  End if
                  End if

                  the bold sections are the ones that immediately drew the attention of the team, but we then looked at the logic itself, and were stunned by the loop itself For Each obj In Request.Form which not only is never used, but just increases the number of iterations of the same code for absolutely no reason whatsoever... Anyone else have the joy of such discoveries?? Any clangers found that left you slack-jawed and slightly disoriented?? :) :)

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

                  Often such strange things are the result of tweaking the code until it finally worked and not taking the time to clean up the debris from all this trial and error.

                  A while ago he asked me what he should have printed on my business cards. I said 'Wizard'. I read books which nobody else understand. Then I do something which nobody understands. After that the computer does something which nobody understands. When asked, I say things about the results which nobody understand. But everybody expects miracles from me on a regular basis. Looks to me like the classical definition of a wizard.

                  T 1 Reply Last reply
                  0
                  • H Hosey

                    I often find myself having to review code within all of our systems as our company grows/changes, as im sure most of us have to during the course of work. Sometimes I find myself looking at old code, and feeling embaressed for the coder (who is sometimes myself) as code that was applicable due to knowledge at the time looks frightful in the light of newer knowledge and skills, but occasionally I come across absolute gems. The coder who wrote this (who shall remain unamed) is no longer with the company, but I do not regret the loss one bit in the face of massive amounts of code being uncovered of this stunning calibre; Classic ASP:

                    If Request.ServerVariables("REQUEST_METHOD") = "POST" Then
                    If Request.Form("btnAddTask") <> "" Then
                    For Each obj In Request.Form

                    iSectionID_New = ""
                    If Len(iSectionID_New) = 0 Then

                    iSectionID_New = Trim(Request.Form("section_id_filter"))
                    End If

                    iTaskID_New = ""
                    If Len(iTaskID_New) = 0 Then

                    iTaskID_New = Trim(Request.Form("task_new"))
                    End If

                    iDisplayOrder_New = ""
                    If Len(iDisplayOrder_New) = 0 Then

                    If Len(Trim(Request.Form("display_order_new"))) > 0 And IsNumeric(Trim(Request.Form("display_order_new"))) Then
                    iDisplayOrder_New = Trim(Request.Form("display_order_new"))
                    End If
                    End If

                    iActionOrder_New = ""
                    If Len(iActionOrder_New) = 0 Then

                    If Len(Trim(Request.Form("action_order_new"))) > 0 And IsNumeric(Trim(Request.Form("action_order_new"))) Then
                    iActionOrder_New = Trim(Request.Form("action_order_new"))
                    End If
                    End If
                    Next
                    End if
                    End if

                    the bold sections are the ones that immediately drew the attention of the team, but we then looked at the logic itself, and were stunned by the loop itself For Each obj In Request.Form which not only is never used, but just increases the number of iterations of the same code for absolutely no reason whatsoever... Anyone else have the joy of such discoveries?? Any clangers found that left you slack-jawed and slightly disoriented?? :) :)

                    G Offline
                    G Offline
                    George Carmichael
                    wrote on last edited by
                    #20

                    I'm astounded that people will criticize code, but make spelling mistakes during their public diatribe ("embaressed").

                    GC

                    H 1 Reply Last reply
                    0
                    • L Lost User

                      Often such strange things are the result of tweaking the code until it finally worked and not taking the time to clean up the debris from all this trial and error.

                      A while ago he asked me what he should have printed on my business cards. I said 'Wizard'. I read books which nobody else understand. Then I do something which nobody understands. After that the computer does something which nobody understands. When asked, I say things about the results which nobody understand. But everybody expects miracles from me on a regular basis. Looks to me like the classical definition of a wizard.

                      T Offline
                      T Offline
                      Tomz_KV
                      wrote on last edited by
                      #21

                      True. In many cases, if a page shows up without a significant delay, its code might not be looked at again.

                      TOMZ_KV

                      L 1 Reply Last reply
                      0
                      • H Hosey

                        I often find myself having to review code within all of our systems as our company grows/changes, as im sure most of us have to during the course of work. Sometimes I find myself looking at old code, and feeling embaressed for the coder (who is sometimes myself) as code that was applicable due to knowledge at the time looks frightful in the light of newer knowledge and skills, but occasionally I come across absolute gems. The coder who wrote this (who shall remain unamed) is no longer with the company, but I do not regret the loss one bit in the face of massive amounts of code being uncovered of this stunning calibre; Classic ASP:

                        If Request.ServerVariables("REQUEST_METHOD") = "POST" Then
                        If Request.Form("btnAddTask") <> "" Then
                        For Each obj In Request.Form

                        iSectionID_New = ""
                        If Len(iSectionID_New) = 0 Then

                        iSectionID_New = Trim(Request.Form("section_id_filter"))
                        End If

                        iTaskID_New = ""
                        If Len(iTaskID_New) = 0 Then

                        iTaskID_New = Trim(Request.Form("task_new"))
                        End If

                        iDisplayOrder_New = ""
                        If Len(iDisplayOrder_New) = 0 Then

                        If Len(Trim(Request.Form("display_order_new"))) > 0 And IsNumeric(Trim(Request.Form("display_order_new"))) Then
                        iDisplayOrder_New = Trim(Request.Form("display_order_new"))
                        End If
                        End If

                        iActionOrder_New = ""
                        If Len(iActionOrder_New) = 0 Then

                        If Len(Trim(Request.Form("action_order_new"))) > 0 And IsNumeric(Trim(Request.Form("action_order_new"))) Then
                        iActionOrder_New = Trim(Request.Form("action_order_new"))
                        End If
                        End If
                        Next
                        End if
                        End if

                        the bold sections are the ones that immediately drew the attention of the team, but we then looked at the logic itself, and were stunned by the loop itself For Each obj In Request.Form which not only is never used, but just increases the number of iterations of the same code for absolutely no reason whatsoever... Anyone else have the joy of such discoveries?? Any clangers found that left you slack-jawed and slightly disoriented?? :) :)

                        J Offline
                        J Offline
                        Jason Christian
                        wrote on last edited by
                        #22

                        I think my favorite was an Access program (yes, there are things worse than VB), where most of the logic was in nested IIfs within queries. After copying them into a text editor so I could separate them out enough to see what was going on, I discovered that the last 3 or 4 of the 10-15 nested IIfs all =0 for the true result. There were 4-5 query columns like this. So the last 3 or 4 conditions didn't need to be checked because they would all result in 0. If a, then 0, else if b, then 0, else if c then 0. Lovely code.

                        H 1 Reply Last reply
                        0
                        • D Dalek Dave

                          I once came upon a huge line that contained about 15 nested if then else statements. Not only was it ugly, it was very inefficient. Horrid little bit of code, but it worked, although owing to the nature of what it was doing, a simple Case would have been infinitely preferable. There was no reason to do it that way, and I told myself off!

                          ------------------------------------ I will never again mention that I was the poster of the One Millionth Lounge Post, nor that it was complete drivel. Dalek Dave

                          K Offline
                          K Offline
                          Kirk Wood
                          wrote on last edited by
                          #23

                          For the last 18 months I have been stuck with maintaining a 2500 line if statement. (I have never managed to actually count how deep the layers get.) And, to make matters worse there is no means of unit testing available. Thank goodness the replacement system is in place.

                          1 Reply Last reply
                          0
                          • H Hosey

                            I often find myself having to review code within all of our systems as our company grows/changes, as im sure most of us have to during the course of work. Sometimes I find myself looking at old code, and feeling embaressed for the coder (who is sometimes myself) as code that was applicable due to knowledge at the time looks frightful in the light of newer knowledge and skills, but occasionally I come across absolute gems. The coder who wrote this (who shall remain unamed) is no longer with the company, but I do not regret the loss one bit in the face of massive amounts of code being uncovered of this stunning calibre; Classic ASP:

                            If Request.ServerVariables("REQUEST_METHOD") = "POST" Then
                            If Request.Form("btnAddTask") <> "" Then
                            For Each obj In Request.Form

                            iSectionID_New = ""
                            If Len(iSectionID_New) = 0 Then

                            iSectionID_New = Trim(Request.Form("section_id_filter"))
                            End If

                            iTaskID_New = ""
                            If Len(iTaskID_New) = 0 Then

                            iTaskID_New = Trim(Request.Form("task_new"))
                            End If

                            iDisplayOrder_New = ""
                            If Len(iDisplayOrder_New) = 0 Then

                            If Len(Trim(Request.Form("display_order_new"))) > 0 And IsNumeric(Trim(Request.Form("display_order_new"))) Then
                            iDisplayOrder_New = Trim(Request.Form("display_order_new"))
                            End If
                            End If

                            iActionOrder_New = ""
                            If Len(iActionOrder_New) = 0 Then

                            If Len(Trim(Request.Form("action_order_new"))) > 0 And IsNumeric(Trim(Request.Form("action_order_new"))) Then
                            iActionOrder_New = Trim(Request.Form("action_order_new"))
                            End If
                            End If
                            Next
                            End if
                            End if

                            the bold sections are the ones that immediately drew the attention of the team, but we then looked at the logic itself, and were stunned by the loop itself For Each obj In Request.Form which not only is never used, but just increases the number of iterations of the same code for absolutely no reason whatsoever... Anyone else have the joy of such discoveries?? Any clangers found that left you slack-jawed and slightly disoriented?? :) :)

                            T Offline
                            T Offline
                            the Kris
                            wrote on last edited by
                            #24

                            What about the next piece of code, which was endorsed by the product manager :

                            while ( true)
                            {
                            if( formName == Evita.GuiCommon.Constants.GuiScreens.STARTING_UP)
                            {
                            this.mExeStartingUpHandler.Display();
                            break;
                            }
                            if( formName == Evita.GuiCommon.Constants.GuiScreens.Exe.ADD_CLEANING_STATION)
                            {
                            this.mExeAddCleaningStationHandler.Display(this.mTcpIpMessages);
                            break;
                            }
                            else if( formName == Evita.GuiCommon.Constants.GuiScreens.Exe.ADD_DRIVER)
                            {
                            this.mExeAddDriverHandler.Display(this.mTcpIpMessages);
                            break;
                            }
                            else if (...)
                            ...
                            }

                            The reason for doing the while loop with the breaks : it runs faster than if-else-if without it. Also note there was no final break, so if the loop was entered with an unknown 'formName', it would loop forever. Countless arguments against doing things like this came to my mind, but they were all disregarded and this was the way to do it. Then they wondered why some people started to rebel.

                            1 Reply Last reply
                            0
                            • T Tomz_KV

                              True. In many cases, if a page shows up without a significant delay, its code might not be looked at again.

                              TOMZ_KV

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

                              It has happened to everyone, I guess. Be it that, you had only little time or the next emergency had to be taken care of. Whatever happened, you simply forgot to clean up.

                              A while ago he asked me what he should have printed on my business cards. I said 'Wizard'. I read books which nobody else understand. Then I do something which nobody understands. After that the computer does something which nobody understands. When asked, I say things about the results which nobody understand. But everybody expects miracles from me on a regular basis. Looks to me like the classical definition of a wizard.

                              T 1 Reply Last reply
                              0
                              • L Lost User

                                It has happened to everyone, I guess. Be it that, you had only little time or the next emergency had to be taken care of. Whatever happened, you simply forgot to clean up.

                                A while ago he asked me what he should have printed on my business cards. I said 'Wizard'. I read books which nobody else understand. Then I do something which nobody understands. After that the computer does something which nobody understands. When asked, I say things about the results which nobody understand. But everybody expects miracles from me on a regular basis. Looks to me like the classical definition of a wizard.

                                T Offline
                                T Offline
                                Tomz_KV
                                wrote on last edited by
                                #26

                                Totally agree with you.

                                TOMZ_KV

                                1 Reply Last reply
                                0
                                • G George Carmichael

                                  I'm astounded that people will criticize code, but make spelling mistakes during their public diatribe ("embaressed").

                                  GC

                                  H Offline
                                  H Offline
                                  Hosey
                                  wrote on last edited by
                                  #27

                                  Thats the reason im not a teacher... The code i posted is the reason some people should rethink being a developer ;)

                                  1 Reply Last reply
                                  0
                                  • J Jason Christian

                                    I think my favorite was an Access program (yes, there are things worse than VB), where most of the logic was in nested IIfs within queries. After copying them into a text editor so I could separate them out enough to see what was going on, I discovered that the last 3 or 4 of the 10-15 nested IIfs all =0 for the true result. There were 4-5 query columns like this. So the last 3 or 4 conditions didn't need to be checked because they would all result in 0. If a, then 0, else if b, then 0, else if c then 0. Lovely code.

                                    H Offline
                                    H Offline
                                    Hosey
                                    wrote on last edited by
                                    #28

                                    There are always some gems somewhere... I think back to some of my early code that I found a year or so later, and nearly died on the spot.. :D

                                    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