widelands-dev team mailing list archive

[Klaus Halfmann <klaus.halfmann@xxxxxxxx>]

Review: Approve compile / test

Compiles, did some test with Gun, which foound bug #1542821 but this is the same in trunk.
Maybe some lists will stay in some other state now, but as of the networking error there
is no consistent state anyway.

OTOH we get rid of some memory leaks.


Diff comments:

> 
> === modified file 'src/ui_fsmenu/internet_lobby.cc'
> --- src/ui_fsmenu/internet_lobby.cc	2016-02-07 16:31:06 +0000
> +++ src/ui_fsmenu/internet_lobby.cc	2016-02-19 20:11:01 +0000
> @@ -176,16 +176,18 @@
>  		InternetGaming::ref().handle_metaserver_communication();
>  	}
>  
> -	if (InternetGaming::ref().update_for_clients())
> +	if (InternetGaming::ref().update_for_clients()) {
>  		fill_client_list(InternetGaming::ref().clients());
> +	}
>  

This is a bad pattern for multithreaded Programms:
if (someLengthyExpression->getSomeThingThatMayBeNull()) {
   // Return value may have changed in some other thread
   someLengthyExpression->getSomeThingThatMayBeNull()->doSomething();
}
Better:
SomeThingThatMayBeNull mayBe = someLengthyExpression->getSomeThingThatMayBeNull();
if (mayBe) {
   mayBe->doSomething();
}

C++ may take over new Operators from languangs like swift: 
someLengthyExpression?->getSomeThingThatMayBeNull()?->doSomething()
> https://developer.apple.com/library/ios/documentation/Swift/Conceptual/Swift_Programming_Language/OptionalChaining.html#//apple_ref/doc/uid/TP40014097-CH21-ID245

BUT, for Widelands this OK :-)

> -	if (InternetGaming::ref().update_for_games())
> +	if (InternetGaming::ref().update_for_games()) {
>  		fill_games_list(InternetGaming::ref().games());
> +	}
>  }
>  
>  void FullscreenMenuInternetLobby::clicked_ok()
>  {
> -	if (joingame.enabled()) {
> +	if (joingame_.enabled()) {
>  		server_doubleclicked();
>  	} else {
>  		clicked_hostgame();


-- 
https://code.launchpad.net/~widelands-dev/widelands/network-memory/+merge/286162
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/network-memory.