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. False selection...

False selection...

Scheduled Pinned Locked Moved The Weird and The Wonderful
javagame-devregexlearning
62 Posts 21 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 Stryder_1

    Don't fret about posting. Most of the horror posts don't seem to generate this much discussion. I see that as a good thing.

    L Offline
    L Offline
    lordofawesome
    wrote on last edited by
    #25

    Ye i like the activity on this forum :D

    1 Reply Last reply
    0
    • L lordofawesome

      This is a piece of code i took from a book i read about java game development.

      /**
      Determines if two display modes "match". Two display
      modes match if they have the same resolution, bit depth,
      and refresh rate. The bit depth is ignored if one of the
      modes has a bit depth of DisplayMode.BIT_DEPTH_MULTI.
      Likewise, the refresh rate is ignored if one of the
      modes has a refresh rate of
      DisplayMode.REFRESH_RATE_UNKNOWN.
      */
      public boolean displayModesMatch(DisplayMode mode1,
      DisplayMode mode2)
      {
      if (mode1.getWidth() != mode2.getWidth() ||
      mode1.getHeight() != mode2.getHeight())
      {
      return false;
      }

          if (mode1.getBitDepth() != DisplayMode.BIT\_DEPTH\_MULTI &&
              mode2.getBitDepth() != DisplayMode.BIT\_DEPTH\_MULTI &&
              mode1.getBitDepth() != mode2.getBitDepth())
          {
              return false;
          }
      
          if (mode1.getRefreshRate() !=
              DisplayMode.REFRESH\_RATE\_UNKNOWN &&
              mode2.getRefreshRate() !=
              DisplayMode.REFRESH\_RATE\_UNKNOWN &&
              mode1.getRefreshRate() != mode2.getRefreshRate())
           {
               return false;
           }
      
           return true;
      }
      
      A Offline
      A Offline
      AspDotNetDev
      wrote on last edited by
      #26

      I was always told to only have one return statement in a procedure, so I can see this being a horror.

      [Forum Guidelines]

      OriginalGriffO F 2 Replies Last reply
      0
      • A AspDotNetDev

        I was always told to only have one return statement in a procedure, so I can see this being a horror.

        [Forum Guidelines]

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

        Rules are made to be broken! Especially when you know why you are breaking them :-D (That's why C# still has a "goto" but students are told not to use it.)

        Real men don't use instructions. They are only the manufacturers opinion on how to put the thing together.

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

        A 1 Reply Last reply
        0
        • L Lost User

          It sure does its job, wich itself is not really complicated. So why don't you just post your more fitting version?

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

          S Offline
          S Offline
          supercat9
          wrote on last edited by
          #28

          CDP1802 wrote:

          It sure does its job, wich itself is not really complicated. So why don't you just post your more fitting version?

          How about creating a function to check if any two of the three parameters are equal? Calling that function with a parameter from each display mode along with the "matches anything" constant would perform three 'equals' tests.

          1 Reply Last reply
          0
          • OriginalGriffO OriginalGriff

            Rules are made to be broken! Especially when you know why you are breaking them :-D (That's why C# still has a "goto" but students are told not to use it.)

            Real men don't use instructions. They are only the manufacturers opinion on how to put the thing together.

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

            OriginalGriff wrote:

            That's why C# still has a "goto" but students are told not to use it.

            I always thought it was because the developer who stole modified the C++ compiler forgot to take that part out.

            [Forum Guidelines]

            G 1 Reply Last reply
            0
            • L lordofawesome

              I think this is a code horror because all this could be done with 1 statement, this is the most performant way to do this AND with proper indenting the readability isn't compromised.

              P Offline
              P Offline
              PIEBALDconsult
              wrote on last edited by
              #30

              Ideally, yes, but there are exceptions... this isn't one of them.

              1 Reply Last reply
              0
              • L lordofawesome

                I'd probably go for this approach

                /**
                Determines if two display modes "match". Two display
                modes match if they have the same resolution, bit depth,
                and refresh rate. The bit depth is ignored if one of the
                modes has a bit depth of DisplayMode.BIT_DEPTH_MULTI.
                Likewise, the refresh rate is ignored if one of the
                modes has a refresh rate of
                DisplayMode.REFRESH_RATE_UNKNOWN.
                */
                public boolean displayModesMatch(DisplayMode mode1,
                DisplayMode mode2)
                {
                boolean isEqualDimention = (mode1.getWidth() == mode2.getWidth() && mode1.getHeight() == mode2.getHeight());
                boolean isEqualDepth = (mode1.getBitDepth() == mode2.getBitDepth());
                boolean isEqualRefreshRate = (mode1.getRefreshRate() == mode2.getRefreshRate());

                return (isEqualDimention && isEqualDepth && isEqualRefreshRate);
                }
                

                I think this is much more readable and much simpler.

                E Offline
                E Offline
                ely_bob
                wrote on last edited by
                #31

                you version here is more costly... You have to remember that when your doing a game engine each comparison burns cycles, the initial example i would argue is faster to execute, if you know more about your environment... if the first statement is true, then you cut down on the number of lookups... using your "better code" actually has worse performance... because you force all the lookups! :doh:

                I'd blame it on the Brain farts.. But let's be honest, it really is more like a Methane factory between my ears some days then it is anything else... -"The conversations he was having with himself were becoming ominous."-.. On the radio...

                L 1 Reply Last reply
                0
                • S Stryder_1

                  Hi, One reason for the original approach is it reduces processing time, which is of primary importance for a game engine. Your proposal will require the function to process each variable before returning while the original will check the most likely areas of failure first, then return -- eliminating the need to process the further checks.

                  E Offline
                  E Offline
                  ely_bob
                  wrote on last edited by
                  #32

                  .. argh I should have read all the sub threads before posting.. sorry.

                  I'd blame it on the Brain farts.. But let's be honest, it really is more like a Methane factory between my ears some days then it is anything else... -"The conversations he was having with himself were becoming ominous."-.. On the radio...

                  1 Reply Last reply
                  0
                  • L lordofawesome

                    Assuming it really is faster, this method isn't really part of the engine itself. it's not like this function will be run over and over again. Me personally, i wouldn't compromise readability over such a small performance increasement. But i'm severely starting to doubt myself, a lot of people seem to disagree this is a codehorror :s

                    E Offline
                    E Offline
                    ely_bob
                    wrote on last edited by
                    #33

                    have you ever had time to go get a burger during a loading scene...? :laugh:

                    I'd blame it on the Brain farts.. But let's be honest, it really is more like a Methane factory between my ears some days then it is anything else... -"The conversations he was having with himself were becoming ominous."-.. On the radio...

                    1 Reply Last reply
                    0
                    • L lordofawesome

                      But aren't these simply constants? So the value would still have to be the same right? if so, this would mean that you don't need to take them into account. Or am i missing the point :s

                      E Offline
                      E Offline
                      ely_bob
                      wrote on last edited by
                      #34

                      not necessarily, you have mip-maping, or dynamic scaling, or in game render target changes...

                      I'd blame it on the Brain farts.. But let's be honest, it really is more like a Methane factory between my ears some days then it is anything else... -"The conversations he was having with himself were becoming ominous."-.. On the radio...

                      1 Reply Last reply
                      0
                      • L lordofawesome

                        I'd probably go for this approach

                        /**
                        Determines if two display modes "match". Two display
                        modes match if they have the same resolution, bit depth,
                        and refresh rate. The bit depth is ignored if one of the
                        modes has a bit depth of DisplayMode.BIT_DEPTH_MULTI.
                        Likewise, the refresh rate is ignored if one of the
                        modes has a refresh rate of
                        DisplayMode.REFRESH_RATE_UNKNOWN.
                        */
                        public boolean displayModesMatch(DisplayMode mode1,
                        DisplayMode mode2)
                        {
                        boolean isEqualDimention = (mode1.getWidth() == mode2.getWidth() && mode1.getHeight() == mode2.getHeight());
                        boolean isEqualDepth = (mode1.getBitDepth() == mode2.getBitDepth());
                        boolean isEqualRefreshRate = (mode1.getRefreshRate() == mode2.getRefreshRate());

                        return (isEqualDimention && isEqualDepth && isEqualRefreshRate);
                        }
                        

                        I think this is much more readable and much simpler.

                        J Offline
                        J Offline
                        James H
                        wrote on last edited by
                        #35

                        Your code also checks things it needn't - once the first bool is false we know the displays are not the same so we can short circuit and return - why do the extra compares unless you just want to burn a bit of CPU

                        1 Reply Last reply
                        0
                        • L lordofawesome

                          I'd probably go for this approach

                          /**
                          Determines if two display modes "match". Two display
                          modes match if they have the same resolution, bit depth,
                          and refresh rate. The bit depth is ignored if one of the
                          modes has a bit depth of DisplayMode.BIT_DEPTH_MULTI.
                          Likewise, the refresh rate is ignored if one of the
                          modes has a refresh rate of
                          DisplayMode.REFRESH_RATE_UNKNOWN.
                          */
                          public boolean displayModesMatch(DisplayMode mode1,
                          DisplayMode mode2)
                          {
                          boolean isEqualDimention = (mode1.getWidth() == mode2.getWidth() && mode1.getHeight() == mode2.getHeight());
                          boolean isEqualDepth = (mode1.getBitDepth() == mode2.getBitDepth());
                          boolean isEqualRefreshRate = (mode1.getRefreshRate() == mode2.getRefreshRate());

                          return (isEqualDimention && isEqualDepth && isEqualRefreshRate);
                          }
                          

                          I think this is much more readable and much simpler.

                          L Offline
                          L Offline
                          LockH
                          wrote on last edited by
                          #36

                          I think the original version is better because I read it once and was reasonably confident I knew what it did, and also what was intended. The shorter, cleverer version I looked at and said "Eh?". In my first job I discovered that programs might have to survive being "fixed" by a half-drunk trainee who had been dragged from the pub when something went wrong on Friday night at the start of a holiday weekend and all the sober and competent programmers were gone. I know, I was that trainee. OK, so this isn't an IBM 360 COBOL accounting program, but still, clarity of intention is desirable. KISS, in fact.

                          1 Reply Last reply
                          0
                          • E ely_bob

                            you version here is more costly... You have to remember that when your doing a game engine each comparison burns cycles, the initial example i would argue is faster to execute, if you know more about your environment... if the first statement is true, then you cut down on the number of lookups... using your "better code" actually has worse performance... because you force all the lookups! :doh:

                            I'd blame it on the Brain farts.. But let's be honest, it really is more like a Methane factory between my ears some days then it is anything else... -"The conversations he was having with himself were becoming ominous."-.. On the radio...

                            L Offline
                            L Offline
                            lordofawesome
                            wrote on last edited by
                            #37

                            Check my last reply, 1 statement, no variables, only indenting to preserve readability and my personnal favorite

                            E 1 Reply Last reply
                            0
                            • L lordofawesome

                              Check my last reply, 1 statement, no variables, only indenting to preserve readability and my personnal favorite

                              E Offline
                              E Offline
                              ely_bob
                              wrote on last edited by
                              #38

                              I likes it at first glance.... succinct. These calls in my experience can be a bit black box however and although would work fine in situations where compute time isn't at a premium is always the best way to go, i would (at least in my projects) check to see if it is actually faster and produces less garbage (GC type garbage, not the visual type, which it obviously doesn't...). it's also been my experience that a lot of these nice API methods are just wrappers for crappy unoptimized code... the XNA. Vector3d.Distance function is a prime example of this... problem with API functions is that they need to be generic, and work in all situations (even when given properties..etc) and as such often make internal scope storage "inside the black box" to accomplish their result...{actually posed this question to a guy who wrote that function on the XNA team.. this was his answer!} so I see it as elegantly coded, but not necessarily the best approach in all situations..

                              I'd blame it on the Brain farts.. But let's be honest, it really is more like a Methane factory between my ears some days then it is anything else...
                              -----
                              "The conversations he was having with himself were becoming ominous."-.. On the radio...

                              1 Reply Last reply
                              0
                              • L lordofawesome

                                I'd probably go for this approach

                                /**
                                Determines if two display modes "match". Two display
                                modes match if they have the same resolution, bit depth,
                                and refresh rate. The bit depth is ignored if one of the
                                modes has a bit depth of DisplayMode.BIT_DEPTH_MULTI.
                                Likewise, the refresh rate is ignored if one of the
                                modes has a refresh rate of
                                DisplayMode.REFRESH_RATE_UNKNOWN.
                                */
                                public boolean displayModesMatch(DisplayMode mode1,
                                DisplayMode mode2)
                                {
                                boolean isEqualDimention = (mode1.getWidth() == mode2.getWidth() && mode1.getHeight() == mode2.getHeight());
                                boolean isEqualDepth = (mode1.getBitDepth() == mode2.getBitDepth());
                                boolean isEqualRefreshRate = (mode1.getRefreshRate() == mode2.getRefreshRate());

                                return (isEqualDimention && isEqualDepth && isEqualRefreshRate);
                                }
                                

                                I think this is much more readable and much simpler.

                                W Offline
                                W Offline
                                waldemar sauer aitmetis com
                                wrote on last edited by
                                #39

                                This format definitely has an advantage in that if you suspect displayModesMatch of doing something wrong, or otherwise just want to know what it is doing, you only need to set one breakpoint. I guess both approaches have their merits. But before we just go ahead and assume that this way is better than that way, let's see how C++ would have done this task. I found the results quite surprising. To simplify, I used the following code snippet:

                                bool test1(int dim1, int dim2, int depth1, int depth2, int refresh1, int refresh2)
                                {
                                bool isEqDim = (dim1 == dim2);
                                bool isEqDepth = (depth1 == depth2);
                                bool isEqRefresh = (refresh1 == refresh2);
                                return isEqDim && isEqDepth && isEqRefresh;
                                }

                                bool test2(int dim1, int dim2, int depth1, int depth2, int refresh1, int refresh2)
                                {
                                if (dim1 != dim2)
                                return false;
                                if (depth1 != depth2)
                                return false;
                                if (refresh1 != refresh2)
                                return false;
                                return true;
                                }

                                void trialrig()
                                {
                                int dim1, dim2, depth1, depth2, refresh1, refresh2;
                                scanf("%i", &dim1);
                                scanf("%i", &dim2);
                                scanf("%i", &depth1);
                                scanf("%i", &depth2);
                                scanf("%i", &refresh1);
                                scanf("%i", &refresh2);
                                bool b1_1 = test1(dim1, dim2, depth1, depth2, refresh1, refresh2);
                                bool b2_1 = test2(dim1, dim2, depth1, depth2, refresh1, refresh2);
                                if (b1_1)
                                printf("b1 is true\n");
                                if (b2_1)
                                printf("b2 is true\n");
                                }

                                The scanf's were important, because if they're not there, my compiler just optimized both the test functions out of existence. During compiling, some of these were inlined, so I have to compensate somewhat. So for simplicity, assume that both code blocks below are prefixed with these instructions:

                                mov    eax, DWORD PTR \_dim1$\[esp+88\]
                                mov    ecx, DWORD PTR \_dim2$\[esp+88\]
                                mov    esi, DWORD PTR \_depth1$\[esp+88\]
                                mov    edi, DWORD PTR \_depth2$\[esp+88\]
                                mov    ebx, DWORD PTR \_refresh1$\[esp+88\]
                                mov    ebp, DWORD PTR \_refresh2$\[esp+88\]
                                

                                In release build, it produced the following for one of the two functions (excluding the preamble above):

                                cmp    eax, ecx
                                jne    SHORT $LN7@test12
                                cmp    esi, edi
                                jne    SHORT $LN7@test12
                                cmp    ebx, ebp
                                jne    SHORT $LN7@test12
                                mov    dl, 1
                                jmp    SHORT $LN8@test12
                                

                                $LN7@test12:
                                xor dl, dl
                                $LN8@test12:

                                And the following code for the other function:

                                cmp    eax, ecx
                                je
                                
                                L 1 Reply Last reply
                                0
                                • W waldemar sauer aitmetis com

                                  This format definitely has an advantage in that if you suspect displayModesMatch of doing something wrong, or otherwise just want to know what it is doing, you only need to set one breakpoint. I guess both approaches have their merits. But before we just go ahead and assume that this way is better than that way, let's see how C++ would have done this task. I found the results quite surprising. To simplify, I used the following code snippet:

                                  bool test1(int dim1, int dim2, int depth1, int depth2, int refresh1, int refresh2)
                                  {
                                  bool isEqDim = (dim1 == dim2);
                                  bool isEqDepth = (depth1 == depth2);
                                  bool isEqRefresh = (refresh1 == refresh2);
                                  return isEqDim && isEqDepth && isEqRefresh;
                                  }

                                  bool test2(int dim1, int dim2, int depth1, int depth2, int refresh1, int refresh2)
                                  {
                                  if (dim1 != dim2)
                                  return false;
                                  if (depth1 != depth2)
                                  return false;
                                  if (refresh1 != refresh2)
                                  return false;
                                  return true;
                                  }

                                  void trialrig()
                                  {
                                  int dim1, dim2, depth1, depth2, refresh1, refresh2;
                                  scanf("%i", &dim1);
                                  scanf("%i", &dim2);
                                  scanf("%i", &depth1);
                                  scanf("%i", &depth2);
                                  scanf("%i", &refresh1);
                                  scanf("%i", &refresh2);
                                  bool b1_1 = test1(dim1, dim2, depth1, depth2, refresh1, refresh2);
                                  bool b2_1 = test2(dim1, dim2, depth1, depth2, refresh1, refresh2);
                                  if (b1_1)
                                  printf("b1 is true\n");
                                  if (b2_1)
                                  printf("b2 is true\n");
                                  }

                                  The scanf's were important, because if they're not there, my compiler just optimized both the test functions out of existence. During compiling, some of these were inlined, so I have to compensate somewhat. So for simplicity, assume that both code blocks below are prefixed with these instructions:

                                  mov    eax, DWORD PTR \_dim1$\[esp+88\]
                                  mov    ecx, DWORD PTR \_dim2$\[esp+88\]
                                  mov    esi, DWORD PTR \_depth1$\[esp+88\]
                                  mov    edi, DWORD PTR \_depth2$\[esp+88\]
                                  mov    ebx, DWORD PTR \_refresh1$\[esp+88\]
                                  mov    ebp, DWORD PTR \_refresh2$\[esp+88\]
                                  

                                  In release build, it produced the following for one of the two functions (excluding the preamble above):

                                  cmp    eax, ecx
                                  jne    SHORT $LN7@test12
                                  cmp    esi, edi
                                  jne    SHORT $LN7@test12
                                  cmp    ebx, ebp
                                  jne    SHORT $LN7@test12
                                  mov    dl, 1
                                  jmp    SHORT $LN8@test12
                                  

                                  $LN7@test12:
                                  xor dl, dl
                                  $LN8@test12:

                                  And the following code for the other function:

                                  cmp    eax, ecx
                                  je
                                  
                                  L Offline
                                  L Offline
                                  lordofawesome
                                  wrote on last edited by
                                  #40

                                  Wow this information is interesting. Thx for the effort. I could be wrong but i can't see the solution with 1 statement anywhere among your tests functions. I presume that using that solution would be even faster.

                                  W 1 Reply Last reply
                                  0
                                  • L lordofawesome

                                    Wow this information is interesting. Thx for the effort. I could be wrong but i can't see the solution with 1 statement anywhere among your tests functions. I presume that using that solution would be even faster.

                                    W Offline
                                    W Offline
                                    waldemar sauer aitmetis com
                                    wrote on last edited by
                                    #41

                                    It's all in the first block of code. test1() vs. test2(). The examples are a bit contrived, and might not look like the original question anymore. For one thing, I just assume that you magically know the result of "getWidth()", etc. Basically, I try to pit the concept of declaring boolean variables versus returning false early against each other.

                                    L 1 Reply Last reply
                                    0
                                    • W waldemar sauer aitmetis com

                                      It's all in the first block of code. test1() vs. test2(). The examples are a bit contrived, and might not look like the original question anymore. For one thing, I just assume that you magically know the result of "getWidth()", etc. Basically, I try to pit the concept of declaring boolean variables versus returning false early against each other.

                                      L Offline
                                      L Offline
                                      lordofawesome
                                      wrote on last edited by
                                      #42

                                      i could be saying dumb shit here but would it be faster to not use the variables and simply return the whole thing?

                                      W 1 Reply Last reply
                                      0
                                      • L lordofawesome

                                        i could be saying dumb shit here but would it be faster to not use the variables and simply return the whole thing?

                                        W Offline
                                        W Offline
                                        waldemar sauer aitmetis com
                                        wrote on last edited by
                                        #43

                                        It's hard for me to tell what the answer would be inside a Java interpreter. I suspect that the difference will be small. I doubt that declaring local variables vs. not declaring will actually make a difference in C/C++. Local variables get assigned machine registers, and they may actually not require stack locations at all. In fact, with a decent optimizing compiler, the following snippets will compile to the same code:

                                        void fx(int x)
                                        {
                                        doSomethingWithX(x);
                                        }

                                        and

                                        void fx(int x)
                                        {
                                        int descriptionForX = x;
                                        doSomethingWithX(descriptionForX);
                                        }

                                        The reason is that the 'descriptionForX' variable will be coalesced so that it uses the same register as x, and will subsequently be removed by the compiler.

                                        L 1 Reply Last reply
                                        0
                                        • W waldemar sauer aitmetis com

                                          It's hard for me to tell what the answer would be inside a Java interpreter. I suspect that the difference will be small. I doubt that declaring local variables vs. not declaring will actually make a difference in C/C++. Local variables get assigned machine registers, and they may actually not require stack locations at all. In fact, with a decent optimizing compiler, the following snippets will compile to the same code:

                                          void fx(int x)
                                          {
                                          doSomethingWithX(x);
                                          }

                                          and

                                          void fx(int x)
                                          {
                                          int descriptionForX = x;
                                          doSomethingWithX(descriptionForX);
                                          }

                                          The reason is that the 'descriptionForX' variable will be coalesced so that it uses the same register as x, and will subsequently be removed by the compiler.

                                          L Offline
                                          L Offline
                                          lordofawesome
                                          wrote on last edited by
                                          #44

                                          That sounds reasonable to me. Your knowledge seems impressive. Nevertheless I think it is strange to return a boolean from the result of any if structure. It just seems strange to me. Just one remark on the variable/nonvariable matter. I don't know about C/C++ but in java when you combine boolean expressions using the && operator, automatically when 1 value is false the other expression are skipped. That's why i would assume that it has the same advantage of being able to skip part of the calculations. I would think when using the variables, that advantage would be partially lost because you'd split up the calculations, this enlarging the calculations i'd think. This is just theory i'm not sure of this sorry if i'm not clear i'm having trouble expressing myself in english

                                          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