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. Embarrassing code admission of the day (or why C.S. is good for you)

Embarrassing code admission of the day (or why C.S. is good for you)

Scheduled Pinned Locked Moved The Lounge
questioncsharpandroidcomdesign
63 Posts 29 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.
  • E Ennis Ray Lynch Jr

    Pretend the overall logic is entirely sound. The bug below is very subtle and is not a logic bug but a design bug, to make it harder, pretend the overall logic is correct. What is the bug?

    //init the list and fill it
    List fakeList = new List();
    //Find the subtle bug
    while (fakeList.Count > 0) {
    double temp = fakeList[0];
    //..do something
    fakeList.RemoveAt(0);
    }

    Hint: Ok, if it is too hard. Remember what a List is in C# and then remember the specifics of that data structure from intro to programming. Edit: The data structure is correct, and the logic is technically correct but wrong. Another Hint: Run it with a populated list of 100,000 elements and check the timing. There is a particular feature of this data structure that happens with this particular code that one small change would avoid.

    Need custom software developed? I do custom programming based primarily on MS tools with an emphasis on C# development and consulting. I also do Android Programming as I find it a refreshing break from the MS. "And they, since they Were not the one dead, turned to their affairs" -- Robert Frost

    V Offline
    V Offline
    V 0
    wrote on last edited by
    #26

    Sorry, can't find anything wrong. The only thing I see on sight is you manipulate the size of a list while looping it. This is potentially dangerous, depending on what you do with it.

    		Random r = new Random((int) DateTime.Now.Ticks);			
    		List fakelist = new List();
    		
    		Console.WriteLine("Populating list");
    		for(int i = 0; i < 100000; i++){
    			fakelist.Add(r.NextDouble() + i);
    		}												//end for
    		
    		Console.WriteLine("looping list, writing to file");
    		System.IO.StreamWriter writer = new System.IO.StreamWriter(@"C:\\temp\\fakelist.txt");
    		int index = 0;
    		while(fakelist.Count > 0){
    			double temp = fakelist\[0\];
    			writer.Write(temp);
    			writer.Write("\\t");
    			if(index%10 == 0){
    				writer.WriteLine("");
    				writer.WriteLine(DateTime.Now.ToString("dd/MM/yyyy HH:mm:ss:ffff"));
    				writer.WriteLine("");
    			}											//end if
    			writer.Flush();
    			index++;
    			fakelist.RemoveAt(0);
    		}												//end while
    		Console.WriteLine("Done!");
    		writer.Close();			
    
    		Console.WriteLine("Press enter to quit.");
    		Console.ReadLine();
    

    V.

    V 1 Reply Last reply
    0
    • M Mladen Jankovic

      Unless the actual implementation of the algorithm starts iterating from the end of the list, from some strange reason.

      J Offline
      J Offline
      Julien Villers
      wrote on last edited by
      #27

      Well, in .NET a List is well documented, so list[0] should be O(1), while list[n-1] is also 0(1) (see post below for MSDN link). A .NET List is supposed to be a dynamic array, with the expected performance of an array for single element access. So reversing the loop shown here would not even cause the access speed to be bad, but even if it were bad, it would not be as bad as the constant resizing (RemoveAt(0)) would be. Morality: use a Queue (or not).

      'As programmers go, I'm fairly social. Which still means I'm a borderline sociopath by normal standards.' Jeff Atwood 'I'm French! Why do you think I've got this outrrrrageous accent?' Monty Python and the Holy Grail

      M X 2 Replies Last reply
      0
      • P Pete OHanlon

        Took me a moment or two to spot that. Couldn't really see it until I thought it through. Good catch - how did you find it? For others - what happens when you remove at 0? How is this handled in terms of resizing when you remove from the start of the list. As a comparison, remove from the last position instead (ok, it's not the same logical code, but it shows timings).

        *pre-emptive celebratory nipple tassle jiggle* - Sean Ewington

        "Mind bleach! Send me mind bleach!" - Nagy Vilmos

        My blog | My articles | MoXAML PowerToys | Mole 2010 - debugging made easier - my favourite utility

        M Offline
        M Offline
        Mladen Jankovic
        wrote on last edited by
        #28

        I'm not familiar with internals of List<> in .Net. I was under assumption that it is implemented as plain vanilla list in which case the removal of either head or tail should consume the same amount of time, but your hint tells me that it isn't usual list.

        1 Reply Last reply
        0
        • _ _Zorro_

          Oh, what would be a better approach? ElementAt? I thought it would be the same...

          G Offline
          G Offline
          GParkings
          wrote on last edited by
          #29

          IIRC its an extension method so very likely to be the same as it will be using the pre-existing public members unless of course there's some IL magic going on...

          Pedis ex oris Quidquid latine dictum sit, altum sonatur

          1 Reply Last reply
          0
          • X Xiangyang Liu

            Ennis Ray Lynch, Jr. wrote:

            Another Hint: Run it with a populated list of 100,000 elements and check the timing. There is a particular feature of this data structure that happens with this particular code that one small change would avoid.

            But that is not called a bug, is it?

            My Younger Son & His "PET"

            T Offline
            T Offline
            TheGreatAndPowerfulOz
            wrote on last edited by
            #30

            Technically, it "works". But, if you also define working as being the most reasonably efficient, then yes it it is a bug.

            If your actions inspire others to dream more, learn more, do more and become more, you are a leader." - John Quincy Adams
            You must accept one of two basic premises: Either we are alone in the universe, or we are not alone in the universe. And either way, the implications are staggering” - Wernher von Braun

            1 Reply Last reply
            0
            • D Dario Solera

              No, it's O(1) (because there's an array behind): http://msdn.microsoft.com/en-us/library/0ebtbkkc.aspx[^]

              If you truly believe you need to pick a mobile phone that "says something" about your personality, don't bother. You don't have a personality. A mental illness, maybe, but not a personality. [Charlie Brooker] ScrewTurn Wiki, Software Localization Tools & Services and My Blog

              M Offline
              M Offline
              Mladen Jankovic
              wrote on last edited by
              #31

              Well I was under wrong impession that List<> is implemented as linked list and not array. Blame it on C++ and STL :)

              1 Reply Last reply
              0
              • E Ennis Ray Lynch Jr

                Pretend the overall logic is entirely sound. The bug below is very subtle and is not a logic bug but a design bug, to make it harder, pretend the overall logic is correct. What is the bug?

                //init the list and fill it
                List fakeList = new List();
                //Find the subtle bug
                while (fakeList.Count > 0) {
                double temp = fakeList[0];
                //..do something
                fakeList.RemoveAt(0);
                }

                Hint: Ok, if it is too hard. Remember what a List is in C# and then remember the specifics of that data structure from intro to programming. Edit: The data structure is correct, and the logic is technically correct but wrong. Another Hint: Run it with a populated list of 100,000 elements and check the timing. There is a particular feature of this data structure that happens with this particular code that one small change would avoid.

                Need custom software developed? I do custom programming based primarily on MS tools with an emphasis on C# development and consulting. I also do Android Programming as I find it a refreshing break from the MS. "And they, since they Were not the one dead, turned to their affairs" -- Robert Frost

                T Offline
                T Offline
                TheGreatAndPowerfulOz
                wrote on last edited by
                #32

                Assuming this is C#, removing the first item in a list creates an entirely new list, which I think is a bug in the underlying list code since removing the first item should just rebase the list head at the next item. I think the underlying list code uses an array, believe it or not. It's not a "bug" per se, unless you consider taking a very long time to do a simple operation a bug.

                If your actions inspire others to dream more, learn more, do more and become more, you are a leader." - John Quincy Adams
                You must accept one of two basic premises: Either we are alone in the universe, or we are not alone in the universe. And either way, the implications are staggering” - Wernher von Braun

                C 1 Reply Last reply
                0
                • E Ennis Ray Lynch Jr

                  Pretend the overall logic is entirely sound. The bug below is very subtle and is not a logic bug but a design bug, to make it harder, pretend the overall logic is correct. What is the bug?

                  //init the list and fill it
                  List fakeList = new List();
                  //Find the subtle bug
                  while (fakeList.Count > 0) {
                  double temp = fakeList[0];
                  //..do something
                  fakeList.RemoveAt(0);
                  }

                  Hint: Ok, if it is too hard. Remember what a List is in C# and then remember the specifics of that data structure from intro to programming. Edit: The data structure is correct, and the logic is technically correct but wrong. Another Hint: Run it with a populated list of 100,000 elements and check the timing. There is a particular feature of this data structure that happens with this particular code that one small change would avoid.

                  Need custom software developed? I do custom programming based primarily on MS tools with an emphasis on C# development and consulting. I also do Android Programming as I find it a refreshing break from the MS. "And they, since they Were not the one dead, turned to their affairs" -- Robert Frost

                  S Offline
                  S Offline
                  Sentenryu
                  wrote on last edited by
                  #33

                  if you are looking for performance, use a for loop with a control variable and after the loop, call List.Clear(); you'll be saving numerous function calls... remove the first element each time it's not necessary nor benefic.

                  I'm brazilian and english (well, human languages in general) aren't my best skill, so, sorry by my english. (if you want we can speak in C# or VB.Net =p)

                  1 Reply Last reply
                  0
                  • E Ennis Ray Lynch Jr

                    Pretend the overall logic is entirely sound. The bug below is very subtle and is not a logic bug but a design bug, to make it harder, pretend the overall logic is correct. What is the bug?

                    //init the list and fill it
                    List fakeList = new List();
                    //Find the subtle bug
                    while (fakeList.Count > 0) {
                    double temp = fakeList[0];
                    //..do something
                    fakeList.RemoveAt(0);
                    }

                    Hint: Ok, if it is too hard. Remember what a List is in C# and then remember the specifics of that data structure from intro to programming. Edit: The data structure is correct, and the logic is technically correct but wrong. Another Hint: Run it with a populated list of 100,000 elements and check the timing. There is a particular feature of this data structure that happens with this particular code that one small change would avoid.

                    Need custom software developed? I do custom programming based primarily on MS tools with an emphasis on C# development and consulting. I also do Android Programming as I find it a refreshing break from the MS. "And they, since they Were not the one dead, turned to their affairs" -- Robert Frost

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

                    Removing at the beginning is the stupidest thing to do, but not actually a bug is it? A performance regression sure..

                    1 Reply Last reply
                    0
                    • J Julien Villers

                      Well, in .NET a List is well documented, so list[0] should be O(1), while list[n-1] is also 0(1) (see post below for MSDN link). A .NET List is supposed to be a dynamic array, with the expected performance of an array for single element access. So reversing the loop shown here would not even cause the access speed to be bad, but even if it were bad, it would not be as bad as the constant resizing (RemoveAt(0)) would be. Morality: use a Queue (or not).

                      'As programmers go, I'm fairly social. Which still means I'm a borderline sociopath by normal standards.' Jeff Atwood 'I'm French! Why do you think I've got this outrrrrageous accent?' Monty Python and the Holy Grail

                      M Offline
                      M Offline
                      Mladen Jankovic
                      wrote on last edited by
                      #35

                      Julien Villers wrote:

                      A .NET List is supposed to be a dynamic array, with the expected performance of an array for single element access.

                      Blame it on C++ and STL, but who in his right mind name class for one data structure after another stucture? :)

                      Julien Villers wrote:

                      while list[n-1] is also 0(1)

                      Also, your original post (now edited) suggests that even you was under implession that List<> is in fact a list not an array ;)

                      J 1 Reply Last reply
                      0
                      • P Pete OHanlon

                        Took me a moment or two to spot that. Couldn't really see it until I thought it through. Good catch - how did you find it? For others - what happens when you remove at 0? How is this handled in terms of resizing when you remove from the start of the list. As a comparison, remove from the last position instead (ok, it's not the same logical code, but it shows timings).

                        *pre-emptive celebratory nipple tassle jiggle* - Sean Ewington

                        "Mind bleach! Send me mind bleach!" - Nagy Vilmos

                        My blog | My articles | MoXAML PowerToys | Mole 2010 - debugging made easier - my favourite utility

                        E Offline
                        E Offline
                        Ennis Ray Lynch Jr
                        wrote on last edited by
                        #36

                        Trust me when I saw it; I laughed.

                        Need custom software developed? I do custom programming based primarily on MS tools with an emphasis on C# development and consulting. I also do Android Programming as I find it a refreshing break from the MS. "And they, since they Were not the one dead, turned to their affairs" -- Robert Frost

                        P 1 Reply Last reply
                        0
                        • M Mladen Jankovic

                          Julien Villers wrote:

                          A .NET List is supposed to be a dynamic array, with the expected performance of an array for single element access.

                          Blame it on C++ and STL, but who in his right mind name class for one data structure after another stucture? :)

                          Julien Villers wrote:

                          while list[n-1] is also 0(1)

                          Also, your original post (now edited) suggests that even you was under implession that List<> is in fact a list not an array ;)

                          J Offline
                          J Offline
                          Julien Villers
                          wrote on last edited by
                          #37

                          Yup, it's not a very good choice (ArrayList was nicer, but not generic). On the other hand, consider all the time gained by typing List<> instead of DynamicArray<> :D

                          'As programmers go, I'm fairly social. Which still means I'm a borderline sociopath by normal standards.' Jeff Atwood 'I'm French! Why do you think I've got this outrrrrageous accent?' Monty Python and the Holy Grail

                          M 1 Reply Last reply
                          0
                          • E Ennis Ray Lynch Jr

                            Pretend the overall logic is entirely sound. The bug below is very subtle and is not a logic bug but a design bug, to make it harder, pretend the overall logic is correct. What is the bug?

                            //init the list and fill it
                            List fakeList = new List();
                            //Find the subtle bug
                            while (fakeList.Count > 0) {
                            double temp = fakeList[0];
                            //..do something
                            fakeList.RemoveAt(0);
                            }

                            Hint: Ok, if it is too hard. Remember what a List is in C# and then remember the specifics of that data structure from intro to programming. Edit: The data structure is correct, and the logic is technically correct but wrong. Another Hint: Run it with a populated list of 100,000 elements and check the timing. There is a particular feature of this data structure that happens with this particular code that one small change would avoid.

                            Need custom software developed? I do custom programming based primarily on MS tools with an emphasis on C# development and consulting. I also do Android Programming as I find it a refreshing break from the MS. "And they, since they Were not the one dead, turned to their affairs" -- Robert Frost

                            A Offline
                            A Offline
                            Andy Brummer
                            wrote on last edited by
                            #38

                            Just the performance thing? If you want to get into the nitty gritty of performance under dot net, what is the impact of something like List vs. List on garbage collection when we are talking millions of records. Discounting the fact that each string probably takes up more memory than a double.

                            1 Reply Last reply
                            0
                            • E Ennis Ray Lynch Jr

                              Trust me when I saw it; I laughed.

                              Need custom software developed? I do custom programming based primarily on MS tools with an emphasis on C# development and consulting. I also do Android Programming as I find it a refreshing break from the MS. "And they, since they Were not the one dead, turned to their affairs" -- Robert Frost

                              P Offline
                              P Offline
                              Pete OHanlon
                              wrote on last edited by
                              #39

                              In the examples you get with the Sync Framework, there's a similar issue from the other side. Basically, the code checks to see if it has already found a file and, if not, it adds it to the end. The problem, of course, is that the more files you add, the longer the find takes to complete for files that aren't present in the list. I know of companies who are using this in production code, little realising why their applications are slowing down.

                              *pre-emptive celebratory nipple tassle jiggle* - Sean Ewington

                              "Mind bleach! Send me mind bleach!" - Nagy Vilmos

                              My blog | My articles | MoXAML PowerToys | Mole 2010 - debugging made easier - my favourite utility

                              1 Reply Last reply
                              0
                              • V V 0

                                Sorry, can't find anything wrong. The only thing I see on sight is you manipulate the size of a list while looping it. This is potentially dangerous, depending on what you do with it.

                                		Random r = new Random((int) DateTime.Now.Ticks);			
                                		List fakelist = new List();
                                		
                                		Console.WriteLine("Populating list");
                                		for(int i = 0; i < 100000; i++){
                                			fakelist.Add(r.NextDouble() + i);
                                		}												//end for
                                		
                                		Console.WriteLine("looping list, writing to file");
                                		System.IO.StreamWriter writer = new System.IO.StreamWriter(@"C:\\temp\\fakelist.txt");
                                		int index = 0;
                                		while(fakelist.Count > 0){
                                			double temp = fakelist\[0\];
                                			writer.Write(temp);
                                			writer.Write("\\t");
                                			if(index%10 == 0){
                                				writer.WriteLine("");
                                				writer.WriteLine(DateTime.Now.ToString("dd/MM/yyyy HH:mm:ss:ffff"));
                                				writer.WriteLine("");
                                			}											//end if
                                			writer.Flush();
                                			index++;
                                			fakelist.RemoveAt(0);
                                		}												//end while
                                		Console.WriteLine("Done!");
                                		writer.Close();			
                                
                                		Console.WriteLine("Press enter to quit.");
                                		Console.ReadLine();
                                

                                V.

                                V Offline
                                V Offline
                                V 0
                                wrote on last edited by
                                #40

                                Random r = new Random((int) DateTime.Now.Ticks);
                                List fakelist = new List();

                                		Console.WriteLine("Populating list");
                                		for(int i = 0; i < 100000; i++){
                                			fakelist.Add(r.NextDouble() + i);
                                		}												//end for
                                		Console.WriteLine("looping list, remove from front.");
                                		DateTime start = DateTime.Now;
                                		while(fakelist.Count > 0){
                                			double temp = fakelist\[0\];
                                			fakelist.RemoveAt(0);
                                		}												//end while
                                
                                		TimeSpan diff = DateTime.Now - start;
                                		Console.WriteLine("Time: " + diff.TotalMilliseconds + " milliseconds.");
                                
                                		Console.WriteLine("Populating list");
                                		for(int i = 0; i < 100000; i++){
                                			fakelist.Add(r.NextDouble() + i);
                                		}												//end for
                                
                                		Console.WriteLine("looping list,removing from back.");
                                		int inverseindex = fakelist.Count-1;
                                		start = DateTime.Now;
                                		while(fakelist.Count > 0){
                                		    double temp = fakelist\[inverseindex\];
                                		    fakelist.RemoveAt(inverseindex);
                                		    inverseindex--;
                                		}												//end while
                                
                                		diff = DateTime.Now - start;
                                		Console.WriteLine("Time: " + diff.TotalMilliseconds + " milliseconds.");
                                
                                		Console.WriteLine("Done!");
                                		Console.WriteLine("Press enter to quit.");
                                		Console.ReadLine();
                                

                                output:

                                Populating list
                                looping list, remove from front.
                                Time: 5468,82 milliseconds.
                                Populating list
                                looping list,removing from back.
                                Time: 0 milliseconds.
                                Done!
                                Press enter to quit.

                                So yes, there is a signifant performance gain. Well spotted. :thumbsup:

                                V.

                                1 Reply Last reply
                                0
                                • J Julien Villers

                                  Yup, it's not a very good choice (ArrayList was nicer, but not generic). On the other hand, consider all the time gained by typing List<> instead of DynamicArray<> :D

                                  'As programmers go, I'm fairly social. Which still means I'm a borderline sociopath by normal standards.' Jeff Atwood 'I'm French! Why do you think I've got this outrrrrageous accent?' Monty Python and the Holy Grail

                                  M Offline
                                  M Offline
                                  Mladen Jankovic
                                  wrote on last edited by
                                  #41

                                  STL uses vector as name for dynamic arrays, that's just two letters more and much less misleading :)

                                  L 1 Reply Last reply
                                  0
                                  • M Mladen Jankovic

                                    STL uses vector as name for dynamic arrays, that's just two letters more and much less misleading :)

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

                                    How is that "much less misleading"? It has "mathematical vector" written all over it, but it's not even close to that. Meanwhile why should a list, sans "linked", be linked?

                                    M 1 Reply Last reply
                                    0
                                    • K Keith Barrow

                                      Ah. Now you see, that is another example as to why programming is hard.

                                      Sort of a cross between Lawrence of Arabia and Dilbert.[^]
                                      -Or-
                                      A Dead ringer for Kate Winslett[^]

                                      P Offline
                                      P Offline
                                      Pete OHanlon
                                      wrote on last edited by
                                      #43

                                      Keith Barrow wrote:

                                      another example as to why programming is hard

                                      Programming is easy. Doing it properly is hard.

                                      *pre-emptive celebratory nipple tassle jiggle* - Sean Ewington

                                      "Mind bleach! Send me mind bleach!" - Nagy Vilmos

                                      My blog | My articles | MoXAML PowerToys | Mole 2010 - debugging made easier - my favourite utility

                                      1 Reply Last reply
                                      0
                                      • L Lost User

                                        How is that "much less misleading"? It has "mathematical vector" written all over it, but it's not even close to that. Meanwhile why should a list, sans "linked", be linked?

                                        M Offline
                                        M Offline
                                        Mladen Jankovic
                                        wrote on last edited by
                                        #44

                                        harold aptroot wrote:

                                        How is that "much less misleading"?

                                        Because it is not in direct collision with another widely used data structure and mathematical vector is not used as much as the other tow (list/array) in programming.

                                        harold aptroot wrote:

                                        Meanwhile why should a list, sans "linked", be linked?

                                        It doesn't have to be, theoretically, but in practice when someone mention list, one usually assume it is a linked list and everything that goes with it (costs/benefits), and thus it is misleading. OP's example illustrates my point and I'll quote the guy before you how later edited his post:

                                        Well, in .NET a List is well documented, so list[0] should be O(1), while list[n-1] should be O(n).

                                        Add me to this list and you have 3 guys that were misled by the class name. Even though some of them were aware of how List<> is actually implemented.

                                        L 1 Reply Last reply
                                        0
                                        • T TheGreatAndPowerfulOz

                                          Assuming this is C#, removing the first item in a list creates an entirely new list, which I think is a bug in the underlying list code since removing the first item should just rebase the list head at the next item. I think the underlying list code uses an array, believe it or not. It's not a "bug" per se, unless you consider taking a very long time to do a simple operation a bug.

                                          If your actions inspire others to dream more, learn more, do more and become more, you are a leader." - John Quincy Adams
                                          You must accept one of two basic premises: Either we are alone in the universe, or we are not alone in the universe. And either way, the implications are staggering” - Wernher von Braun

                                          C Offline
                                          C Offline
                                          Chris Losinger
                                          wrote on last edited by
                                          #45

                                          ahmed zahmed wrote:

                                          removing the first item in a list creates an entirely new list,

                                          :wtf: seriously?

                                          image processing toolkits | batch image processing

                                          P T 2 Replies 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