Hello Monty!
>From bffbd4f20d898ad4a1dbaeaffa2a2f12b7298d73 Mon Sep 17 00:00:00 2001
> From: Monty <[email protected]>
> Date: Tue, 3 Mar 2026 15:26:03 +0200
> Subject: [PATCH] Remove for (auto []) constructs from rpl_master_info.h
I welcome the intent for the code understandability improvement. Actually,
I see a degradation of understandability. My suggestions follow.
diff --git a/sql/rpl_master_info_file.h b/sql/rpl_master_info_file.h
> index 90bc08fb..24cbcf55 100644
> --- a/sql/rpl_master_info_file.h
> +++ b/sql/rpl_master_info_file.h
> @@ -83,7 +83,7 @@ inline const char *master_ssl_key = "";
> inline const char *master_ssl_cipher = "";
> inline bool master_ssl_verify_server_cert= true;
> /// `ulong` is the data type `my_getopt` expects.
> -inline auto master_use_gtid=
> static_cast<ulong>(enum_master_use_gtid::DEFAULT);
> +inline ulong master_use_gtid=
> static_cast<ulong>(enum_master_use_gtid::DEFAULT);
> inline uint64_t master_retry_count= 100000;
> /// }@
>
> @@ -612,9 +612,11 @@ struct Master_info_file: Info_file
> ignore_server_ids(ignore_server_ids),
> do_domain_ids(do_domain_ids), ignore_domain_ids(ignore_domain_ids)
> {
> - for(auto &[_, mem_fn]: VALUE_MAP)
> - if (mem_fn)
> - mem_fn(this).set_default();
> + for (std::unordered_map<std::string_view,
> + const Mem_fn>::const_iterator it= VALUE_MAP.begin();
>
It is a very long type specialization for being used inline. Actually, it's
three times longer, that the action made. Also it adds zero context.
The real action is:
assign VALUE_MAP.begin() to it;
What to do:
Declare a typedef for this particular iterator:
typedef std::unordered_map<std::string_view, const Mem_fn>::const_iterator
VALUE_MAP_iterator;
> + it != VALUE_MAP.end(); ++it)
> + if (it->second)
>
it is very unclear, what 'second' means. I should hold a context to
remember that it's an iterator.
Also, if the type changes to non-stl container, this code will have to be
totally rewritten another time.
Declare the local variable with the nema hinting on its function and assign
it->second to it
> + it->second(this).set_default();
}
>
> bool load_from_file() override
> @@ -695,12 +697,15 @@ break_for:;
> /* Write MariaDB `key=value` lines:
> The "value" can then be written individually after generating
> the`key=`.
> */
> - for (auto &[key, pm]: VALUE_MAP)
> - if (pm)
> + for (std::unordered_map<std::string_view,
> + const Mem_fn>::const_iterator it= VALUE_MAP.begin();
> + it != VALUE_MAP.end(); ++it)
>
With declaring a typedef a repeated bloat will disappear.
Best regards!
Nikita
_______________________________________________
developers mailing list -- [email protected]
To unsubscribe send an email to [email protected]
