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.
  • 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"

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

    Yes it is! http://www.codinghorror.com/blog/2011/06/performance-is-a-feature.html[^]

    '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

    1 Reply Last reply
    0
    • C Claude Martel Olivier

      Not sure if it's intended or not but you're going to delete the list by deleting the first item over and over?

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

      Basically, one way or the other there's a 0(n) operation - either in finding the element at position n, or removing the element at position n. Removing the element at n where n = 0 would, at first glance, appear to be a good optimisation. Unfortunately, it has side effects.

      *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
      • J Julien Villers

        When n = 0, you could have an exponential cost, it wouldn't matter much, now would it?

        '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
        #24

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

        J 1 Reply Last reply
        0
        • M Mladen Jankovic

          Yes it is, But also it has O(n) complexity.

          D Offline
          D Offline
          Dario Solera
          wrote on last edited by
          #25

          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 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

            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
                          • 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
                            #34

                            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
                            • 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
                              #35

                              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
                              • 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
                                • 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
                                  #37

                                  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
                                  • 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
                                    #38

                                    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

                                      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
                                          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