[PATCH xorg-gtest 1/4] environment: provide Get/SetFoobar wrappers for all set/get_foobar
Chase Douglas
chase.douglas at canonical.com
Wed Jul 11 12:41:34 PDT 2012
On 07/10/2012 06:51 PM, Peter Hutterer wrote:
> The google coding style endorses set_foo and get_foo for setters/getters,
> but DoSomething style for anything else. Let's not do that, stick to one
> CamelCase coding style only since most of the functions we'll call will
> likely be CamelCase.
>
> Having a separate style for getters/setters gets ambiguous when functions
> are more complex than simple assignment. e.g. should it be
> add_to_display_string() or AddToDisplayString()?
> To avoid ambiguity and confusion, enforce one, CamelCase, style only.
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
> include/xorg/gtest/xorg-gtest-environment.h | 67 +++++++++++++++++++++++----
> src/environment.cpp | 61 +++++++++++++++++++-----
> src/xorg-gtest_main.cpp | 8 ++--
> 3 files changed, 111 insertions(+), 25 deletions(-)
>
> diff --git a/include/xorg/gtest/xorg-gtest-environment.h b/include/xorg/gtest/xorg-gtest-environment.h
> index de680ca..aee9cdd 100644
> --- a/include/xorg/gtest/xorg-gtest-environment.h
> +++ b/include/xorg/gtest/xorg-gtest-environment.h
> @@ -80,14 +80,14 @@ class Environment : public ::testing::Environment {
> *
> * @param path_to_log_file Path to server logfile.
> */
> - void set_log_file(const std::string& path_to_log_file);
> + void SetLogFile(const std::string& path_to_log_file);
>
> /**
> * Returns the path where the server log file will be created.
> *
> * @return Path to server logfile.
> */
> - const std::string& log_file() const;
> + const std::string& GetLogFile() const;
>
> /**
> * Sets the path to the desired server configuration file.
> @@ -95,16 +95,16 @@ class Environment : public ::testing::Environment {
> * The path will be passed on to the server via the command line argument
> * "-config". The default value is "[datadir]/xorg/gtest/dummy.conf".
> *
> - * @param path_conf_file Path to a Xorg X server .conf file.
> + * @param path_to_conf_file Path to a Xorg X server .conf file.
> */
> - void set_conf_file(const std::string& path_conf_file);
> + void SetConfigFile(const std::string& path_to_conf_file);
>
> /**
> * Returns the path of the server configuration file to be used.
> *
> * @return File path of the server configuration currently set
> */
> - const std::string& conf_file() const;
> + const std::string& GetConfigFile() const;
>
> /**
> * Sets the path to the server executable
> @@ -113,14 +113,14 @@ class Environment : public ::testing::Environment {
> *
> * @param path_to_server Path to an X.org server executable
> */
> - void set_server(const std::string& path_to_server);
> + void SetServerPath(const std::string& path_to_server);
>
> /**
> * Returns the path of the server executable to be used.
> *
> * @return Path to server executable.
> */
> - const std::string& server() const;
> + const std::string& GetServerPath() const;
>
> /**
> * Sets the display number that the server will use.
> @@ -130,20 +130,69 @@ class Environment : public ::testing::Environment {
> *
> * @param display_num A display number.
> */
> - void set_display(int display_num);
> + void SetDisplayNumber(int display_num);
>
> /**
> * Returns the display number of the server instance.
> *
> * @return Display number of the server.
> */
> - int display() const;
> + int GetDisplayNumber() const;
>
> /**
> * Kill the dummy Xorg server with SIGKILL.
> */
> void Kill();
>
> + /* DEPRECATED */
> + /**
> + * @deprecated
> + * @see SetLogFile
> + */
> + void set_log_file(const std::string& path_to_log_file);
> +
> + /**
> + * @deprecated
> + * @see SetLogFile
> + */
> + const std::string& log_file() const;
> +
> + /**
> + * @deprecated
> + * @see SetConfigFile
> + */
> + void set_conf_file(const std::string& path_conf_file);
> +
> + /**
> + * @deprecated
> + * @see GetConfigFile
> + */
> + const std::string& conf_file() const;
> +
> + /**
> + * @deprecated
> + * @see SetServerPath
> + */
> + void set_server(const std::string& path_to_server);
> +
> + /**
> + * @deprecated
> + * @see GetServerPath
> + */
> + const std::string& server() const;
> +
> + /**
> + * @deprecated
> + * @see SetDisplay
> + */
> + void set_display(int display_num);
> +
> + /**
> + * @deprecated
> + * @see GetDisplayNumber()
> + */
> + int display() const;
> +
> protected:
> /**
> * Starts the dummy X server.
> diff --git a/src/environment.cpp b/src/environment.cpp
> index c29d4a8..b02a6a2 100644
> --- a/src/environment.cpp
> +++ b/src/environment.cpp
> @@ -63,44 +63,44 @@ xorg::testing::Environment::~Environment() {}
>
> void xorg::testing::Environment::set_log_file(const std::string& path_to_log_file)
> {
> + SetLogFile(path_to_log_file);
> +}
> +
> +void xorg::testing::Environment::SetLogFile(const std::string& path_to_log_file)
> +{
> d_->path_to_log_file = path_to_log_file;
> }
>
> -const std::string& xorg::testing::Environment::log_file() const
> +const std::string& xorg::testing::Environment::GetLogFile() const
> {
> return d_->path_to_log_file;
> }
>
> -void xorg::testing::Environment::set_conf_file(const std::string& path_conf_file)
> +void xorg::testing::Environment::SetConfigFile(const std::string& path_to_conf_file)
> {
> - d_->path_to_conf = path_conf_file;
> + d_->path_to_conf = path_to_conf_file;
> }
>
> -const std::string& xorg::testing::Environment::conf_file() const
> +const std::string& xorg::testing::Environment::GetConfigFile() const
> {
> return d_->path_to_conf;
> }
>
> -void xorg::testing::Environment::set_server(const std::string& path_to_server)
> +void xorg::testing::Environment::SetServerPath(const std::string& path_to_server)
> {
> d_->path_to_server = path_to_server;
> }
>
> -const std::string& xorg::testing::Environment::server() const
> +const std::string& xorg::testing::Environment::GetServerPath() const
> {
> return d_->path_to_server;
> }
>
> -void xorg::testing::Environment::set_display(int display_num)
> +void xorg::testing::Environment::SetDisplayNumber(int display_num)
> {
> d_->display = display_num;
> }
>
> -int xorg::testing::Environment::display() const
> -{
> - return d_->display;
> -}
> -
> void xorg::testing::Environment::SetUp() {
> static char display_string[6];
> snprintf(display_string, 6, ":%d", d_->display);
> @@ -215,3 +215,40 @@ void xorg::testing::Environment::Kill() {
>
> std::cerr << "Warning: Dummy X server did not shut down\n";
> }
> +
> +
> +/* DEPRECATED */
> +const std::string& xorg::testing::Environment::log_file() const
> +{
> + return GetLogFile();
> +}
> +
> +void xorg::testing::Environment::set_conf_file(const std::string& path_conf_file)
> +{
> + return SetConfigFile(path_conf_file);
> +}
> +
> +const std::string& xorg::testing::Environment::conf_file() const
> +{
> + return GetConfigFile();
> +}
> +
> +void xorg::testing::Environment::set_server(const std::string& path_to_server)
> +{
> + SetServerPath(path_to_server);
> +}
> +
> +const std::string& xorg::testing::Environment::server() const
> +{
> + return GetServerPath();
> +}
> +
> +void xorg::testing::Environment::set_display(int display_num)
> +{
> + SetDisplayNumber(display_num);
> +}
> +
> +int xorg::testing::Environment::display() const
> +{
> + return d_->display;
> +}
> diff --git a/src/xorg-gtest_main.cpp b/src/xorg-gtest_main.cpp
> index 22c9049..05f63f4 100644
> --- a/src/xorg-gtest_main.cpp
> +++ b/src/xorg-gtest_main.cpp
> @@ -170,16 +170,16 @@ int main(int argc, char *argv[]) {
> environment = new xorg::testing::Environment;
>
> if (xorg_conf_specified)
> - environment->set_conf_file(xorg_conf_path);
> + environment->SetConfigFile(xorg_conf_path);
>
> if (server_specified)
> - environment->set_server(server);
> + environment->SetServerPath(server);
>
> if (xorg_display_specified)
> - environment->set_display(xorg_display);
> + environment->SetDisplayNumber(xorg_display);
>
> if (xorg_logfile_specified)
> - environment->set_log_file(xorg_log_file_path);
> + environment->SetLogFile(xorg_log_file_path);
Uneven indentation with the rest ^^. Perhaps we should run the
formatting script on this. Hopefully it doesn't barf due to C++.
>
> testing::AddGlobalTestEnvironment(environment);
> }
I like how you have clearly deprecated the methods. Thanks for taking
this on.
Reviewed-by: Chase Douglas <chase.douglas at canonical.com>
More information about the xorg-devel
mailing list