GPWiki.org
GPWiki.org
It is currently Wed May 22, 2013 4:59 pm

All times are UTC




Post new topic Reply to topic  [ 26 posts ]  Go to page Previous  1, 2
Author Message
 Post subject:
PostPosted: Thu Jan 22, 2009 8:20 pm 
Sir Postalot
User avatar

Joined: Thu Aug 26, 2004 4:34 am
Posts: 2498
Code:
{ const unsigned lim = 10u;
for(unsigned i = 1, j; i <= lim; ++i)
   for(j = 1u; j <= lim; ++j)
      //if (j == 3u) break; //would run this line full 10 times
      if( j == 3u )
         i = lim; //will only run this thrice
}


Brace wrapping to limit the scope of lim to the pertinent area. I would normally throw that into the for loop (see next bulletpoint) but here the compiler can take advantage of its const attribute and optimize.
Moving j into the first for declaration prevents it from being generated and destroyed (automatic storage class) each i loop.
Unsigned because we are not interested in negative numbers.
Prefix operator can be faster.
Setting i to the limit is effective here, but I would prefer a boolean flag if this were a general case, since it would fail on lim = std::numeric_limits<int type>::max(), where it would roll over to 0 or std::numeric_limits<int type>::min() on the increment, making the mathematics undefined and liable to hurt someone. However, the true sin is the use of <=. Any time you have a foo <= or >= bar in a foor loop, you risk having an all foo is true situation. Sanitize your limit so it must be in the data type's range and that lim is not a useful value of your incrementer. If that cannot be done, for example you may want to run all uint8_t values from 0 to 255, you'll need something else to catch the end condition, such as incrementing a uint16_t and testing & 0x100).


Top
 Profile  
 
 Post subject:
PostPosted: Fri Jan 23, 2009 9:22 am 
Technomaniac

Joined: Sun Dec 05, 2004 11:27 am
Posts: 3249
Location: Sydney, Australia
ViX44 wrote:
Code:
{ const unsigned lim = 10u;
for(unsigned i = 1, j; i <= lim; ++i)
   for(j = 1u; j <= lim; ++j)
      //if (j == 3u) break; //would run this line full 10 times
      if( j == 3u )
         i = lim; //will only run this thrice
}


Brace wrapping to limit the scope of lim to the pertinent area. I would normally throw that into the for loop (see next bulletpoint) but here the compiler can take advantage of its const attribute and optimize.
Moving j into the first for declaration prevents it from being generated and destroyed (automatic storage class) each i loop.
Unsigned because we are not interested in negative numbers.
Prefix operator can be faster.
Setting i to the limit is effective here, but I would prefer a boolean flag if this were a general case, since it would fail on lim = std::numeric_limits<int type>::max(), where it would roll over to 0 or std::numeric_limits<int type>::min() on the increment, making the mathematics undefined and liable to hurt someone. However, the true sin is the use of <=. Any time you have a foo <= or >= bar in a foor loop, you risk having an all foo is true situation. Sanitize your limit so it must be in the data type's range and that lim is not a useful value of your incrementer. If that cannot be done, for example you may want to run all uint8_t values from 0 to 255, you'll need something else to catch the end condition, such as incrementing a uint16_t and testing & 0x100).

A few things:
1) Limiting the scope of variables is good. However, don't do it for performance reasons (your compiler can see that you aren't using it elsewhere).
2) The cost of creating/destroying an automatic variable without a constructor/destructor is zero.
3) Unsigned operations are the same speed as signed operations on every platform ever :)
4) ++i will compile to the same thing as i++ if you aren't actually making use of the return value.

I don't want to be the person that is always saying "just let your compiler do it, it is smarter than you", because I don't believe that is true (a significant part of my work now is hand compiling stuff that compilers do a sucky job of). But doing tiny tweaks that could be done automatically by a computer program is a waste of your time.
The example is a bit contrived, so this ends up looking fairly verbose for something that doesn't do much. But this is how I would approach it:
Code:
typedef bool(*ConditionFunc)(Position p);
typedef struct {
  int i;
  int j;
} *Position;

bool condition(Position p) {
  return (p->j == 3);
}

Position find_position(ConditionFunc filter) {
  Position search = new Position();

  for (search->i = 0; search->i <= 10; search->i++) {
    for (search->j = 0; search->j <= 10; search->j++) {
      if (filter(search)) {
        return search;
      }
    }
  }
  delete(search);
  return NULL;
}

...
blah = find_position(condition);

_________________
Trying is the first step towards failure
b


Top
 Profile  
 
 Post subject:
PostPosted: Fri Jan 23, 2009 12:53 pm 
Digerati

Joined: Fri Sep 24, 2004 3:15 am
Posts: 1791
Location: No longer near Boston, MA
I guess the only real way to make it pretty is to write an extra search function, which is sad.

If you templatized find_position then you could do stuff with functors and lambda, like find_position(DELAY(GetPosition, _1, 2) == 3) so you can write the conditions where you use them... I think there HAS to be a more elegant solution, though.


Top
 Profile  
 
 Post subject:
PostPosted: Fri Jan 23, 2009 1:05 pm 
Technomaniac

Joined: Sun Dec 05, 2004 11:27 am
Posts: 3249
Location: Sydney, Australia
Sadistic Penguin wrote:
I guess the only real way to make it pretty is to write an extra search function, which is sad.

More functions = more fun.

Seriously, what's the problem with more functions? (assuming they can be given sensible names - which is the case here)

_________________
Trying is the first step towards failure
b


Top
 Profile  
 
 Post subject:
PostPosted: Fri Jan 23, 2009 1:58 pm 
Digerati

Joined: Fri Sep 24, 2004 3:15 am
Posts: 1791
Location: No longer near Boston, MA
It depends on what you're doing with them. If you're just writing simple search code for every single type of data container you have you'd think there'd be a way to generalize, saving time/effort/bugs. Simple lambda functions also look better for simple conditions, imo.

Here's a better solution -

Code:
template <int depth>
struct DepthFirstSearch  //Technically isn't exactly DFS, but it works the same way
{
template <typename T, typename Fun>
static inline typename FindNestedIterator<T, depth>::Result find(T& Container, const Fun& Condition, bool& found/*many hacks can go here to make this better*/)
{
  for (typename T::iterator iter = Container.begin(); iter != Container.end(); ++iter)
  {
     typename FindNestedIterator<T, depth>::Result res = DepthFirstSearch<depth-1>::find(*iter, Condition, found);
     if (found)  return res;
  }
}
//Something to get a bad iterator here or something
}
template <>
struct DepthFirstSearch<0>
{
  template <typename T, typename Fun>
  static inline typename T::iterator find(T& Container, const Fun& Condition, bool& found)
  {
    for (typename T::iterator iter = Container.begin(); iter != Container.end(); ++iter)
    {
       if (Condition(*iter))
       {
         found = true;
         return res;
       }
    }
    return Container.end();
  }
  }
};

//Usage
bool found = false;
std::vector<MapTile>::iterator iter = DepthFirstSearch<1>::find(MapTiles, _1 ->* &MapTile::j == 3, found);
if (found) {...} else {...}  //Edit: fixed a stupid naming problem



Last edited by Sadistic Penguin on Fri Jan 23, 2009 6:38 pm, edited 1 time in total.

Top
 Profile  
 
 Post subject:
PostPosted: Fri Jan 23, 2009 6:33 pm 
Sir Postalot
User avatar

Joined: Thu Aug 26, 2004 4:34 am
Posts: 2498
1) Limiting the scope of variables is good. However, don't do it for performance reasons (your compiler can see that you aren't using it elsewhere).
That's fine, I'm doing it so I don't try to use it elsewhere.

2) The cost of creating/destroying an automatic variable without a constructor/destructor is zero.
But the code might be put into a template and then fed an object. I'll do it right the first time.

3) Unsigned operations are the same speed as signed operations on every platform ever
Not a speed thing. Negative values are undefined here so a signed type is inappropriate.

4) ++i will compile to the same thing as i++ if you aren't actually making use of the return value.
Of course, but same as #2, I'll do it right the first time.

"But doing tiny tweaks that could be done automatically by a computer program is a waste of your time."

And that is why we learn to code the optimal line when we first write it! Imagine a program written by some poor, poor newbie, "it's just a tiny tweak, a waste of time" through thousands of sub-optimal lines.

Suddenly, I try to open a word processor on a multi-core 3.4GHz system and it takes many times longer than WordPad did on a 0.1GHz Pentium 1. PROGRESS!

Even a nanosecond is huge if you have a billion +1ns suboptimal lines of code being called.


Top
 Profile  
 
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 26 posts ]  Go to page Previous  1, 2

All times are UTC


Who is online

Users browsing this forum: No registered users and 1 guest


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum

Search for:
Jump to:  
Powered by phpBB® Forum Software © phpBB Group