[PATCH xorg-gtest 1/4] environment: provide Get/SetFoobar wrappers for all set/get_foobar

[Chase Douglas]

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>