Code review comment for lp:~abreu-alexandre/unity/dbus-stick-unstick-launcher-icon

Marco Trevisan (TreviƱo) (3v1n0) wrote :

22 + std::string app_uri;
23 + if (desktop_path.empty())
24 + return app_uri;

You can just return "" in that case, without initializing app_uri.
Also at the end of the function returning a temporary it's fine anyway..

46 +Controller::Impl::OnLauncherUpdateIconStickyState(std::string const& icon_uri,
47 + bool sticky)

Please, keep everything in one-line.

61 + AbstractLauncherIcon::Ptr existing_icon_entry =
62 + GetIconByUri(target_uri);

One line, plus use const& (or auto const&).

72 + SortAndUpdate();

Please avoid to do this, if nothing has been actually changed.

86 + else
87 + favorite_store.RemoveFavorite(target_uri);

Add brackets here too please. However in that case, when can happen that a favorite has not a related application icon? Going in edge cases it's ok, anyway.

140 + ASSERT_FALSE(favorite_store.IsFavorite(app_icon->RemoteUri()));

This is quite redundant as it should be guarantee by other tests we have.

« Back to merge proposal