refactor: prefer explicit operator bool() rather implicit and oper.!()

C++11 allows it, fixing the need of safe bool idiom https://stackoverflow.com/questions/6242768/is-the-safe-bool-idiom-obsolete-in-c11 https://en.wikibooks.org/wiki/More_C%2B%2B_Idioms/Safe_bool https://stackoverflow.com/questions/22003102/overloading-logical-not-operator-vs-bool-type-cast-operator
synfig · Oct 11, 2023 · 32cf979 · 32cf979
1 parent 7d33fd8
commit 32cf979
Show file tree

Hide file tree

Showing 10 changed files with 27 additions and 35 deletions.
diff --git a/ETL/ETL/_handle.h b/ETL/ETL/_handle.h
@@ -221,7 +221,7 @@ class handle
 		{ assert(obj); return obj; }
 
 	//! More explicit bool cast
-	operator bool()const
+	explicit operator bool() const noexcept
 		{ return obj != nullptr; }
 
 	operator handle<const value_type>()const
@@ -254,10 +254,6 @@ class handle
 	//! Returns pointer to the object that is being wrapped
 	pointer get()const { return obj; }
 
-	bool
-	operator!()const
-		{ return !obj; }
-
 	//! static_cast<> overload -- Useful for implicit casts
 	template <class U>
 	operator handle<U>()const
@@ -649,12 +645,8 @@ class loose_handle
 	pointer get()const { return obj; }
 
 	//! More explicit bool cast
-	operator bool()const
-		{ return obj!=0; }
-
-	bool
-	operator!()const
-		{ return !obj; }
+	explicit operator bool() const noexcept
+		{ return obj != nullptr; }
 
 	template<typename U>
 	bool type_is() const

diff --git a/synfig-core/src/synfig/rendering/surface.h b/synfig-core/src/synfig/rendering/surface.h
@@ -219,7 +219,8 @@ class SurfaceResource: public etl::shared_object
 		bool convert(const Surface::Token::Handle &token, bool create = true, bool any = false) {
 			if (!resource) return false;
 			if (lock_token && token != this->token) return false;
-			return surface = resource->get_surface(token, exclusive, full, rect, create, any);
+			surface = resource->get_surface(token, exclusive, full, rect, create, any);
+			return static_cast<bool>(surface);
 		}
 
 		template<typename T>
@@ -241,8 +242,8 @@ class SurfaceResource: public etl::shared_object
 			{ return etl::handle<T>::cast_dynamic(surface); }
 		TypeSurface* get_surface() const
 			{ return surface.get(); }
-		operator bool() const
-			{ return surface; }
+		explicit operator bool() const noexcept
+			{ return static_cast<bool>(surface); }
 	};
 
 	typedef LockBase<const Surface, false, false> LockReadBase;

diff --git a/synfig-studio/src/gui/actionmanagers/layeractionmanager.cpp b/synfig-studio/src/gui/actionmanagers/layeractionmanager.cpp
@@ -717,7 +717,7 @@ bool LayerActionManager::query_user_about_foreign_exported_value_nodes(Canvas::H
 					local_canvas_value_node = canvas->find_value_node(modified_id, true);
 				} catch (...) {
 				}
-				const bool link_to_local_canvas = local_canvas_value_node;
+				const bool link_to_local_canvas = static_cast<bool>(local_canvas_value_node);
 				if (link_to_local_canvas) {
 					valuenode_replacements[foreign_value_node] = std::pair<ValueNode::Handle, std::string>(local_canvas_value_node, "");
 				} else {

diff --git a/synfig-studio/src/gui/canvasview.h b/synfig-studio/src/gui/canvasview.h
@@ -640,7 +640,7 @@ class CanvasView : public Dockable, public etl::shared_object
 
 	void preview_option() {on_preview_option();}
 
-	bool is_playing() { return ducks_playing_lock; }
+	bool is_playing() const { return static_cast<bool>(ducks_playing_lock); }
 
 	//! Toggle given handle type
 	//! \Param[in]  type The Duckmatic::Type to toggle

diff --git a/synfig-studio/src/gui/docks/dock_soundwave.cpp b/synfig-studio/src/gui/docks/dock_soundwave.cpp
@@ -199,7 +199,7 @@ class Grid_SoundWave : public Gtk::Grid {
 	bool import_file(const synfig::filesystem::Path& filename) {
 		std::string errors, warnings;
 
-		bool ok = canvas_interface->import(filename, errors, warnings);
+		bool ok = static_cast<bool>(canvas_interface->import(filename, errors, warnings));
 
 		if (!errors.empty())
 			App::dialog_message_1b(

diff --git a/synfig-studio/src/gui/widgets/widget_timetrack.cpp b/synfig-studio/src/gui/widgets/widget_timetrack.cpp
@@ -961,7 +961,7 @@ bool Widget_Timetrack::fetch_waypoints(const WaypointItem &wi, std::set<synfig::
 		if (node)
 			synfig::waypoint_collect(waypoint_set, wi.time_point.get_time(), node, true);
 
-		return node;
+		return static_cast<bool>(node);
 	} catch (std::out_of_range& ex) {
 		synfig::error(_("Timetrack: trying to fetch waypoints of not-mapped parameter: %s [%f]"), wi.path.to_string().c_str(), wi.time_point.get_time());
 	} catch (...) {

diff --git a/synfig-studio/src/synfigapp/actions/layeraddframe.cpp b/synfig-studio/src/synfigapp/actions/layeraddframe.cpp
@@ -36,7 +36,6 @@
 #include <synfig/general.h>
 
 #include "layeraddframe.h"
-#include "layercopy.h"
 #include <synfig/context.h>
 #include <synfigapp/canvasinterface.h>
 #include <synfigapp/instance.h>
@@ -111,7 +110,7 @@ Action::LayerAddFrame::set_param(const synfig::String& name, const Action::Param
 			if (!layer_base)
 				layer_switch = nullptr;
 		}
-		return layer_switch;
+		return static_cast<bool>(layer_switch);
 	}
 	if(name=="time" && param.get_type()==Param::TYPE_TIME)
 	{

diff --git a/synfig-studio/src/synfigapp/actions/valuedescbonesetparent.cpp b/synfig-studio/src/synfigapp/actions/valuedescbonesetparent.cpp
@@ -226,7 +226,7 @@ Action::ValueDescBoneSetParent::prepare()
 
 	// checks if user mixed skeleton bone and skeleton deformation bone and prevents it
 	const auto skeleton_deformation_new_parent_bone_pair = get_bone_pair_composite(value_desc);
-	const bool parent_has_sibbling_bone = skeleton_deformation_new_parent_bone_pair;
+	const bool parent_has_sibbling_bone = static_cast<bool>(skeleton_deformation_new_parent_bone_pair);
 
 	std::pair<ValueNode_Bone::Handle,ValueNode_Bone::Handle> child_bone_pair;
 	const bool child_has_sibbling_bone = get_sibbling_bone_pair(active_bone_, time, child_bone_pair);

diff --git a/synfig-studio/src/synfigapp/value_desc.h b/synfig-studio/src/synfigapp/value_desc.h
@@ -237,11 +237,11 @@ class ValueDesc
 	operator bool()const { return is_valid(); }
 
 	bool
-	parent_is_layer()const
-		{ return layer; }
+	parent_is_layer() const
+		{ return static_cast<bool>(layer); }
 	bool
-	parent_is_value_node()const
-		{ return parent_value_node; }
+	parent_is_value_node() const
+		{ return static_cast<bool>(parent_value_node); }
 	bool
 	parent_is_linkable_value_node()const
 		{ return parent_is_value_node() && index>=0; }
@@ -252,8 +252,8 @@ class ValueDesc
 	parent_is_waypoint()const
 		{ return parent_is_value_node() && index==IS_WAYPOINT; }
 	bool
-	parent_is_canvas()const
-		{ return canvas; }
+	parent_is_canvas() const
+		{ return static_cast<bool>(canvas); }
 	bool
 	parent_is_value_desc()const
 		{ return !sub_names.empty(); }

diff --git a/synfig-studio/test/app_layerduplicate.cpp b/synfig-studio/test/app_layerduplicate.cpp
@@ -246,8 +246,8 @@ static void test_synfigapp_layerduplicate_encapsulated_layer_with_contents()
 	ASSERT_EQUAL("circle", src_canvas->front()->get_name())
 	ASSERT_EQUAL("rectangle", src_canvas->back()->get_name())
 
-	ASSERT_NOT_EQUAL(dup_canvas->front(), src_canvas->front())
-	ASSERT_NOT_EQUAL(dup_canvas->back(), src_canvas->back())
+	ASSERT_FALSE(dup_canvas->front() == src_canvas->front())
+	ASSERT_FALSE(dup_canvas->back() == src_canvas->back())
 }
 
 // Check if duplicated layer_duplicate has its (auto-exported) 'index' parameter renamed
@@ -600,8 +600,8 @@ static void test_synfigapp_layerduplicate_skeleton_with_bone_link()
 	ASSERT_EQUAL("skeleton", canvas->front()->get_name())
 	ASSERT_EQUAL("circle", (*std::next(canvas->begin(),1))->get_name())
 	ASSERT_EQUAL("circle", canvas->back()->get_name())
-	ASSERT_EQUAL(circle, canvas->back())
-	ASSERT_NOT_EQUAL(circle, (*std::next(canvas->begin(),1)))
+	ASSERT(circle == canvas->back())
+	ASSERT_FALSE(circle == *std::next(canvas->begin(), 1))
 
 	// check if bones were duplicated too: they would be automatically with different names
 	ASSERT_EQUAL("my bone 0", (*std::next(canvas->begin(),2))->get_param("bones").get_list_of(synfig::Bone())[0].get_name())
@@ -614,7 +614,7 @@ static void test_synfigapp_layerduplicate_skeleton_with_bone_link()
 	auto cloned_origin = cloned_origin_pair->second;
 	ASSERT_EQUAL("bone_link", cloned_origin->get_name())
 	auto cloned_bone_link = synfig::ValueNode_BoneLink::Handle::cast_static(cloned_origin);
-	ASSERT_NOT_EQUAL(bone_link, cloned_bone_link)
+	ASSERT_FALSE(bone_link == cloned_bone_link)
 
 	ASSERT_EQUAL("constant", bone_link->get_link("bone")->get_name())
 	auto bone_link_const = synfig::ValueNode_Const::Handle::cast_static(bone_link->get_link("bone"));
@@ -714,8 +714,8 @@ static void test_synfigapp_layerduplicate_skeleton_with_animated_bone_link()
 	ASSERT_EQUAL("skeleton", canvas->front()->get_name())
 	ASSERT_EQUAL("circle", (*std::next(canvas->begin(),1))->get_name())
 	ASSERT_EQUAL("circle", canvas->back()->get_name())
-	ASSERT_EQUAL(circle, canvas->back())
-	ASSERT_NOT_EQUAL(circle, (*std::next(canvas->begin(),1)))
+	ASSERT(circle == canvas->back())
+	ASSERT_FALSE(circle == *std::next(canvas->begin(), 1))
 
 	// check if bones were duplicated too: they would be automatically with different names
 	ASSERT_EQUAL("my bone 0", (*std::next(canvas->begin(),2))->get_param("bones").get_list_of(synfig::Bone())[0].get_name())
@@ -731,7 +731,7 @@ static void test_synfigapp_layerduplicate_skeleton_with_animated_bone_link()
 	auto cloned_origin = cloned_origin_pair->second;
 	ASSERT_EQUAL("bone_link", cloned_origin->get_name())
 	auto cloned_bone_link = synfig::ValueNode_BoneLink::Handle::cast_static(cloned_origin);
-	ASSERT_NOT_EQUAL(bone_link, cloned_bone_link)
+	ASSERT_FALSE(bone_link == cloned_bone_link)
 
 	ASSERT_EQUAL("animated", bone_link->get_link("bone")->get_name())
 	auto bone_link_animated = synfig::ValueNode_Animated::Handle::cast_static(bone_link->get_link("bone"));