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. Other Discussions
  3. The Weird and The Wonderful
  4. Code Optimize

Code Optimize

Scheduled Pinned Locked Moved The Weird and The Wonderful
comcode-review
53 Posts 26 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.
  • S Snorri Kristjansson

    Agree. There is another VERY important reason why you should NOT change working production code unless you really must; Don't change tried and tested code! Ok. this code sample you show us is a bit clumsy but it works, right? So why change it? IMHO a good programmer must learnt NOT to change production code unless it's really really needed.

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

    I definitely disagree with you there. Cleaning up code results in a (marginally, for any particular instance, but it builds up) nicer codebase to work in and that results in better productivity for everyone on the team. If it's 'tried and tested' then you can check that the tests still pass and therefore be sure you haven't broken anything with your cleanup.

    S 1 Reply Last reply
    0
    • R Rajesh Anuhya

      Good, every one saying about the readability and Debugging, However this is a Small code, which is called by many classes. my point is why should i declare a extra variable "resultflag", where this method called 5000+ times in every 10 seconds.

      my Tip/Tricks[^] | "Rajesh-Puli" now "Rajesh-Anuhya"

      R Offline
      R Offline
      Rob Grainger
      wrote on last edited by
      #16

      I shouldn't worry about it, if you're accessing a data layer any saving from avoiding allocating 32-bits on the stack is negligible. Premature optimisation is the root of all evil. The only actual "improvement" you've made is that the code occupies less space on screen. All the "improvements" you claim will be performed by a decent optimising compiler on release code anyway. Design for readability/maintainability, then optimise as you have evidence it is worthwhile.

      1 Reply Last reply
      0
      • B BobJanova

        I definitely disagree with you there. Cleaning up code results in a (marginally, for any particular instance, but it builds up) nicer codebase to work in and that results in better productivity for everyone on the team. If it's 'tried and tested' then you can check that the tests still pass and therefore be sure you haven't broken anything with your cleanup.

        S Offline
        S Offline
        Snorri Kristjansson
        wrote on last edited by
        #17

        I agree - Cleaning up the codebase is very tempting, it's much nicer to work on "clean" code. But my point is this: Cleaning up production code just for the sake of making it look "nice" is very dangerous because (depending on code size of course) you WILL create new bugs in doing so.

        B H 2 Replies Last reply
        0
        • S Snorri Kristjansson

          I agree - Cleaning up the codebase is very tempting, it's much nicer to work on "clean" code. But my point is this: Cleaning up production code just for the sake of making it look "nice" is very dangerous because (depending on code size of course) you WILL create new bugs in doing so.

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

          You won't if the code is properly tested, or if you can demonstrate equivalence for all inputs (often not as hard as it sounds).

          1 Reply Last reply
          0
          • S Snorri Kristjansson

            I agree - Cleaning up the codebase is very tempting, it's much nicer to work on "clean" code. But my point is this: Cleaning up production code just for the sake of making it look "nice" is very dangerous because (depending on code size of course) you WILL create new bugs in doing so.

            H Offline
            H Offline
            Harley L Pebley
            wrote on last edited by
            #19

            Snorri wrote:

            But my point is this: Cleaning up production code just for the sake of making it look "nice" is very dangerous because you SOMETIMES will create new bugs in doing so.

            FTFY. :-) And sometimes cleaning up code will fix latent bugs that no one has run into yet, or possibly they've hit them and just haven't reported them, or possibly they've hit them and thought that was normal behavior. Creating new bugs will be mitigated with unit tests; which of course one always has before refactoring.

            S 1 Reply Last reply
            0
            • R Rajesh Anuhya

              Good, every one saying about the readability and Debugging, However this is a Small code, which is called by many classes. my point is why should i declare a extra variable "resultflag", where this method called 5000+ times in every 10 seconds.

              my Tip/Tricks[^] | "Rajesh-Puli" now "Rajesh-Anuhya"

              M Offline
              M Offline
              mrchief_2000
              wrote on last edited by
              #20

              Declaring an extra variable doesn't affect your performance at all. Compilers can (and will) easily do the code reduction you did (note what you did is not an optimization). On the other, having that extra variable does aid in readability and debugging. The only rewrite I would is (if I absolutely have to change something for no reason other than style):

              return ResultFlag != 0;

              K 1 Reply Last reply
              0
              • R Rajesh Anuhya

                Good, every one saying about the readability and Debugging, However this is a Small code, which is called by many classes. my point is why should i declare a extra variable "resultflag", where this method called 5000+ times in every 10 seconds.

                my Tip/Tricks[^] | "Rajesh-Puli" now "Rajesh-Anuhya"

                J Offline
                J Offline
                jschell
                wrote on last edited by
                #21

                Rajesh Anuhya wrote:

                However this is a Small code, which is called by many classes.
                 
                my point is why should i declare a extra variable "resultflag", where this method called 5000+ times in every 10 seconds

                I can only suppose that that is a general statement that has nothing to do with the code presented. First the code presented suggests it is doing a database call. You will not be able to even measure the performance gain that you are claiming because of that. The impact of the database call will completely overwhelm the the measurement of what you are claiming. I would suspect that even the variability of network traffic itself would reduce your measurement to the noise level. Second if your goal is to improve the performance of the that code then you must reduce the number of calls to the database. For example by using a memory cache. That would have a significant impact. Finally the code is using a return value that is going to end up on the stack regardless of whether it is explicitly stated or implicitly stated. I wouldn't be suprised if the emitted code is almost basically the same between the two versions.

                1 Reply Last reply
                0
                • B BobJanova

                  Florin Jurcovici wrote:

                  Although I don't buy either (no ternaries and braces around single statements) - you write the code as is fit initially, and reformat/refactor as needed when you change it.

                  Yes, exactly. And a simple

                  return statement ? a : b

                  ... is not too hard to read, for sure. Someone here is really passive-aggressive anti-ternary, judging by the downvote my other post got :~ Heh, that pattern is even worse.

                  J Offline
                  J Offline
                  James Lonero
                  wrote on last edited by
                  #22

                  Probably gets paid by lines of code.

                  1 Reply Last reply
                  0
                  • R Rajesh Anuhya

                    Today I found this code, from DAL class

                    public Boolean Execute_NoN_Query(string Sqlstring)
                    {
                    int ResultFlag = 0;
                    ResultFlag = MSSqlHelper.SqlHelper.ExecuteNonQuery(SqlServerConnection.Cn, CommandType.Text, Sqlstring);

                              if  (ResultFlag != 0)
                                  return true;
                              else
                                  return false;
                      }
                    

                    My Code is ....

                    public Boolean Execute_NoN_Query(string Sqlstring)
                    {
                    return (0 != MSSqlHelper.SqlHelper.ExecuteNonQuery(SqlServerConnection.Cn, CommandType.Text, Sqlstring));
                    }

                    my Tip/Tricks[^] | "Rajesh-Puli" now "Rajesh-Anuhya"

                    K Offline
                    K Offline
                    KP Lee
                    wrote on last edited by
                    #23

                    I actually came up with your result before I read further and saw your result. Then I thought, No, you(me) are wrong. Sure as shooting, as soon as I code it that way, when the return is 5 call this routine, when less than 0 call another routine otherwise call a third routine, then return true when the result isn't 0. If I make these 5 statements into 2, they'll never ask to do that. :laugh:

                    1 Reply Last reply
                    0
                    • M mrchief_2000

                      Declaring an extra variable doesn't affect your performance at all. Compilers can (and will) easily do the code reduction you did (note what you did is not an optimization). On the other, having that extra variable does aid in readability and debugging. The only rewrite I would is (if I absolutely have to change something for no reason other than style):

                      return ResultFlag != 0;

                      K Offline
                      K Offline
                      KP Lee
                      wrote on last edited by
                      #24

                      Actually, I would move the int declaration down one command, shorten the variable name and use your return with (). I haven't tried it, but I'd be surprised C# would compile your return without ().

                      1 Reply Last reply
                      0
                      • H Harley L Pebley

                        Snorri wrote:

                        But my point is this: Cleaning up production code just for the sake of making it look "nice" is very dangerous because you SOMETIMES will create new bugs in doing so.

                        FTFY. :-) And sometimes cleaning up code will fix latent bugs that no one has run into yet, or possibly they've hit them and just haven't reported them, or possibly they've hit them and thought that was normal behavior. Creating new bugs will be mitigated with unit tests; which of course one always has before refactoring.

                        S Offline
                        S Offline
                        Snorri Kristjansson
                        wrote on last edited by
                        #25

                        Don't show a quote from me that you have changed!!!!!

                        N 1 Reply Last reply
                        0
                        • S Snorri Kristjansson

                          Don't show a quote from me that you have changed!!!!!

                          N Offline
                          N Offline
                          Nagy Vilmos
                          wrote on last edited by
                          #26

                          Don't get upset. FTFY is a common short hand here, it means "Fixed That For You". Whenever a quote is followed by FTFY, the quoter has intentionally changed the text. There are a myriad of reasons for this. Often it is done for humorous effect or, as in this case, to show that the quoter almost agrees with the OP except for a minor difference. [edit] As a member for eight years, I'd assume you would know this.


                          Panic, Chaos, Destruction. My work here is done. Drink. Get drunk. Fall over - P O'H OK, I will win to day or my name isn't Ethel Crudacre! - DD Ethel Crudacre I cannot live by bread alone. Bacon and ketchup are needed as well. - Trollslayer Have a bit more patience with newbies. Of course some of them act dumb - they're often *students*, for heaven's sake - Terry Pratchett

                          L S 2 Replies Last reply
                          0
                          • N Nagy Vilmos

                            Don't get upset. FTFY is a common short hand here, it means "Fixed That For You". Whenever a quote is followed by FTFY, the quoter has intentionally changed the text. There are a myriad of reasons for this. Often it is done for humorous effect or, as in this case, to show that the quoter almost agrees with the OP except for a minor difference. [edit] As a member for eight years, I'd assume you would know this.


                            Panic, Chaos, Destruction. My work here is done. Drink. Get drunk. Fall over - P O'H OK, I will win to day or my name isn't Ethel Crudacre! - DD Ethel Crudacre I cannot live by bread alone. Bacon and ketchup are needed as well. - Trollslayer Have a bit more patience with newbies. Of course some of them act dumb - they're often *students*, for heaven's sake - Terry Pratchett

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

                            Nagy Vilmos wrote:

                            As a member for eight years, I'd presume you would know this.

                            FTFY :)

                            And from the clouds a mighty voice spoke:
                            "Smile and be happy, for it could come worse!"

                            And I smiled and was happy
                            And it came worse.

                            N C 2 Replies Last reply
                            0
                            • L Lost User

                              Nagy Vilmos wrote:

                              As a member for eight years, I'd presume you would know this.

                              FTFY :)

                              And from the clouds a mighty voice spoke:
                              "Smile and be happy, for it could come worse!"

                              And I smiled and was happy
                              And it came worse.

                              N Offline
                              N Offline
                              Nagy Vilmos
                              wrote on last edited by
                              #28

                              CDP1802 wrote:

                              Nagy Vilmos wrote:

                              As a member for eight years, I'd presume you would know this be sober by now.

                              FTFY

                              ftftfyfy


                              Panic, Chaos, Destruction. My work here is done. Drink. Get drunk. Fall over - P O'H OK, I will win to day or my name isn't Ethel Crudacre! - DD Ethel Crudacre I cannot live by bread alone. Bacon and ketchup are needed as well. - Trollslayer Have a bit more patience with newbies. Of course some of them act dumb - they're often *students*, for heaven's sake - Terry Pratchett

                              1 Reply Last reply
                              0
                              • S Snorri Kristjansson

                                Agree. There is another VERY important reason why you should NOT change working production code unless you really must; Don't change tried and tested code! Ok. this code sample you show us is a bit clumsy but it works, right? So why change it? IMHO a good programmer must learnt NOT to change production code unless it's really really needed.

                                J Offline
                                J Offline
                                johannesnestler
                                wrote on last edited by
                                #29

                                I think I have to support your statements downvoted by others - you seem to stand allone with your opinion - Not any longer! @All the theorists with the "well tested code"... I learned the lesson not to change production code "on the fly". Because sometime a piece of code looks stupid or ugly - but it's just an undocumented workarround for a bug in an underlaying system or whatever. It's not always good to assume all other programmers are idiots... So if I change (or better "refactor") production code - this is my intention - it's like adding a new feature, and yes if there are unit tests in place, it helps. But @all the "perfect code" guys: If it is a "well tested code"-project, unit tests exists - how likley is it to find a real coding horror, isn't it more likely to find it in the "not so well tested" code bases (unit tests??? - whats a "unit"?). I view myself as a "perfectionist" while coding, code style nazi, refactoring fan,... - but there is a real world out there where "perfection" most times mean: "Not complete crap" - I worked on tons of codes in my life, ranging in style and effort from "bloody beginner" to "code god" but "academic perfection" I have rarely seen in a non trivial product. Don't get me wrong - I'm always a fan of "better/shorter code" - but some commenters may have to go through some real projects (why not let's say > 1million lines of code), and learn to leave the existing code - if nobody complains about it - alone! :rose:

                                J S 2 Replies Last reply
                                0
                                • R Rajesh Anuhya

                                  Good, every one saying about the readability and Debugging, However this is a Small code, which is called by many classes. my point is why should i declare a extra variable "resultflag", where this method called 5000+ times in every 10 seconds.

                                  my Tip/Tricks[^] | "Rajesh-Puli" now "Rajesh-Anuhya"

                                  J Offline
                                  J Offline
                                  johannesnestler
                                  wrote on last edited by
                                  #30

                                  omg - bad luck with this example for your statement - have a look at the resulting IL - the variable declaration will be optimized away in the release build... If you would realy gain performance...

                                  1 Reply Last reply
                                  0
                                  • R Rajesh Anuhya

                                    Today I found this code, from DAL class

                                    public Boolean Execute_NoN_Query(string Sqlstring)
                                    {
                                    int ResultFlag = 0;
                                    ResultFlag = MSSqlHelper.SqlHelper.ExecuteNonQuery(SqlServerConnection.Cn, CommandType.Text, Sqlstring);

                                              if  (ResultFlag != 0)
                                                  return true;
                                              else
                                                  return false;
                                      }
                                    

                                    My Code is ....

                                    public Boolean Execute_NoN_Query(string Sqlstring)
                                    {
                                    return (0 != MSSqlHelper.SqlHelper.ExecuteNonQuery(SqlServerConnection.Cn, CommandType.Text, Sqlstring));
                                    }

                                    my Tip/Tricks[^] | "Rajesh-Puli" now "Rajesh-Anuhya"

                                    R Offline
                                    R Offline
                                    RobCroll
                                    wrote on last edited by
                                    #31

                                    I would have fixed the argument name "Sqlstring" while I was at it. :)

                                    "You get that on the big jobs."

                                    1 Reply Last reply
                                    0
                                    • T the Kris

                                      If examining the result while debugging is your main concern, you can still write

                                      int ResultFlag = MSSqlHelper.SqlHelper.ExecuteNonQuery(SqlServerConnection.Cn, CommandType.Text, Sqlstring);

                                      return ResultFlag != 0;

                                      I've seen worse though...

                                      const bool valTrue = true;
                                      const bool valFalse = false;

                                      bool doTheWork()
                                      {
                                      ...more impressive code...

                                      if ( result == valTrue ) return true;
                                      else return false;
                                      }

                                      That coder was preparing himself for the time that valTrue became false or something!?

                                      L Offline
                                      L Offline
                                      loctrice
                                      wrote on last edited by
                                      #32

                                      I see that all the time. You have to change doTheWork to workData though

                                      1 Reply Last reply
                                      0
                                      • T the Kris

                                        If examining the result while debugging is your main concern, you can still write

                                        int ResultFlag = MSSqlHelper.SqlHelper.ExecuteNonQuery(SqlServerConnection.Cn, CommandType.Text, Sqlstring);

                                        return ResultFlag != 0;

                                        I've seen worse though...

                                        const bool valTrue = true;
                                        const bool valFalse = false;

                                        bool doTheWork()
                                        {
                                        ...more impressive code...

                                        if ( result == valTrue ) return true;
                                        else return false;
                                        }

                                        That coder was preparing himself for the time that valTrue became false or something!?

                                        A Offline
                                        A Offline
                                        AspDotNetDev
                                        wrote on last edited by
                                        #33

                                        Wow, that is bad. They don't even use an abstract factory pattern to generate valTrue and valFalse. What happens if you want to change how valTrue and valFalse change later on, but without changing the DLL!?

                                        Thou mewling ill-breeding pignut!

                                        1 Reply Last reply
                                        0
                                        • J johannesnestler

                                          I think I have to support your statements downvoted by others - you seem to stand allone with your opinion - Not any longer! @All the theorists with the "well tested code"... I learned the lesson not to change production code "on the fly". Because sometime a piece of code looks stupid or ugly - but it's just an undocumented workarround for a bug in an underlaying system or whatever. It's not always good to assume all other programmers are idiots... So if I change (or better "refactor") production code - this is my intention - it's like adding a new feature, and yes if there are unit tests in place, it helps. But @all the "perfect code" guys: If it is a "well tested code"-project, unit tests exists - how likley is it to find a real coding horror, isn't it more likely to find it in the "not so well tested" code bases (unit tests??? - whats a "unit"?). I view myself as a "perfectionist" while coding, code style nazi, refactoring fan,... - but there is a real world out there where "perfection" most times mean: "Not complete crap" - I worked on tons of codes in my life, ranging in style and effort from "bloody beginner" to "code god" but "academic perfection" I have rarely seen in a non trivial product. Don't get me wrong - I'm always a fan of "better/shorter code" - but some commenters may have to go through some real projects (why not let's say > 1million lines of code), and learn to leave the existing code - if nobody complains about it - alone! :rose:

                                          J Offline
                                          J Offline
                                          Jorgen Andersson
                                          wrote on last edited by
                                          #34

                                          johannesnestler wrote:

                                          I view myself as a "perfectionist" while coding, code style nazi, refactoring fan

                                          You should apply 5 seconds of that into your authoring skills. You really need to work on your paragraphs if you want people to actually read your posts.

                                          Light moves faster than sound. That is why some people appear bright, until you hear them speak. List of common misconceptions

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