widelands-dev team mailing list archive

[Hans Joachim Desserud <launchpad@xxxxxxxxxxxx>]

Hans Joachim Desserud has proposed merging lp:~hjd/widelands/optimizations into lp:widelands.

Requested reviews:
  Widelands Developers (widelands-dev)

For more details, see:
https://code.launchpad.net/~hjd/widelands/optimizations/+merge/101116

I ran utils/create_cppcheck_report and saw it complained a bit about "Possible inefficient checking for 'variable' emptiness." in a few places. This identifies a few places which check for variable.size(), where it looks like !variable.empty() would be equivelant. (Or better, as size() in worst case will count all elements to give an accurate number, while empty only checks whether it has at least one item or not, which would mean O(n) vs O(1) ) I'm not sure how often these methods are called though, so I don't know whether this would be a noticable performance gain. 

Now, c++ is not the language which I'm most proficient in. So to check whether this seems like a good idea, I have pushed this inital patch which fix one file. If people approve of this intial work, I can submit a larger patch which fix more issues in the future.

Also, I'm a bit confused, though this might be due to how cppcheck works. Because I have fixed the two places it complained about in economy/flag.cc, however, size() is used in a similar way at line 72. I guess this isn't listed since it is only a if check, not a while loop which would run the same comparision multiple times. Still, I wonder if checks like this should be changed as well?
-- 
https://code.launchpad.net/~hjd/widelands/optimizations/+merge/101116
Your team Widelands Developers is requested to review the proposed merge of lp:~hjd/widelands/optimizations into lp:widelands.

=== modified file 'src/economy/flag.cc'
--- src/economy/flag.cc	2012-03-23 16:08:40 +0000
+++ src/economy/flag.cc	2012-04-06 15:16:20 +0000
@@ -499,7 +499,7 @@
 */
 void Flag::wake_up_capacity_queue(Game & game)
 {
-	while (m_capacity_wait.size()) {
+	while (!m_capacity_wait.empty()) {
 		Worker * const w = m_capacity_wait[0].get(game);
 		m_capacity_wait.erase(m_capacity_wait.begin());
 		if (w and w->wakeup_flag_capacity(game, *this))
@@ -714,7 +714,7 @@
 {
 	//molog("Flag::cleanup\n");
 
-	while (m_flag_jobs.size()) {
+	while (!m_flag_jobs.empty()) {
 		delete m_flag_jobs.begin()->request;
 		m_flag_jobs.erase(m_flag_jobs.begin());
 	}