From 60b10aa10b48e71024671b6c63b44869ff389f71 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Michal=20Voc=C5=AF?= Date: Fri, 3 Aug 2007 15:15:06 +0000 Subject: [PATCH] applied Francesco G.'s recommendations --- org.glite.jobid.api-cpp/interface/JobId.h | 298 +++++++++++------------------- 1 file changed, 108 insertions(+), 190 deletions(-) diff --git a/org.glite.jobid.api-cpp/interface/JobId.h b/org.glite.jobid.api-cpp/interface/JobId.h index 1acd005..b84590a 100755 --- a/org.glite.jobid.api-cpp/interface/JobId.h +++ b/org.glite.jobid.api-cpp/interface/JobId.h @@ -1,28 +1,7 @@ -// remove protected interface and clear -// remove constructor with string port -// add internal accessors to C struct fields (and use them to avoid unnecessary allocation) -// defaults for the three param constructor (and remove default constructor), make this one explicit -// init variables at definition -// if should be followed by block, & at type -// check operator= for self-assignment -// return ENOMEM and throw std::bad_alloc -// use unique.empty() rather than unique.size() -// throw exception for JobId(NULL) -// base Exception on runtime_error: -// we can then construct the exception with a message of the format: -// "JobId: bad argument ()" -// the invalid argument/arguments can be -// -// -// 0 -// I would call the exception JobIdError -// add inline to definitions - #ifndef GLITE_JOBID_JOBID_H #define GLITE_JOBID_JOBID_H #include -#include #include #include "glite/jobid/cjobid.h" @@ -33,83 +12,33 @@ namespace jobid { /** - * class glite::jobid::Exception + * class glite::jobid::JobIdError */ - -class Exception : public std::runtime_error { + +class JobIdError : public std::runtime_error { public: /** Constructor for mandatory fields. * * Updates all the mandatory fields and names the exception. - * \param[in] source Source filename where the exception was raised. - * \param[in] line_number Line in the source that caused the exception. - * \param[in] method Name of the method that raised the exception. - * \param[in] code Error code giving the reason for exception. * \param[in] exception Error message describing the exception. */ - Exception(const std::string& source, - int line_number, - const std::string& method, - int code, - const std::string& exception) - : source_file(source), line(line_number), error_code(code), - std::runtime_error(formatMessage(exception, method, source, line_number)) - {} + JobIdError(std::string const& exception) + : std::runtime_error(formatMessage(exception)) + {} - - /** Constructor for mandatory fields and the exception chain. - * - * Updates all the mandatory fields, names the exception and - * adds the original exception's error message to the current - * one. - * \param[in] source Source filename where the exception was raised. - * \param[in] line_number Line in the source that caused the exception. - * \param[in] method Name of the method that raised the exception. - * \param[in] code Error code giving the reason for exception. - * \param[in] exception Error message describing the exception. - * \param[in] exc Originally raised exception. - */ - Exception(const std::string& source, - int line_number, - const std::string& method, - int code, - const std::string& exception, - const Exception &exc) - : source_file(source), line(line_number), error_code(code), - std::runtime_error(formatMessage(exception, method, source, line_number) + - exc.what()) - { } - - virtual ~Exception() throw() - {} - -protected: - /** The name of the file where the exception was raised */ - std::string source_file; - /** line number where the exception was raised */ - int line; - /** the name of the method where the exception was raised */ - std::string method_name ; - /** integer error code representing the cause of the error */ - int error_code; + virtual ~JobIdError() throw() + {} +private: /** Format message for this particular exception. * * Returns formatted string describing exception. * \param[in] exception Error message describing the exception. - * \param[in] method Name of the method that raised the exception. - * \param[in] source Source filename where the exception was raised. - * \param[in] line_number Line in the source that caused the exception. */ - std::string formatMessage(const std::string& exception, - const std::string& method, - const std::string& source, - int line) { + std::string formatMessage(std::string const& exception) { std::ostringstream o; - o << "glite.jobid.Exception: " << exception << std::endl - << "\tat " << method << "[" << source << ":" << line << "]" - << std::endl; + o << "JobId: bad argument ( " << exception << ")"; return o.str(); } }; @@ -137,7 +66,7 @@ public: * @param job_id_string * @throws Exception When a string is passed in a wrong format */ - JobId(const std::string& job_id_string); + JobId(std::string const& job_id_string); /** * Constructor from job id components. @@ -145,8 +74,9 @@ public: * \param port * \param unique */ - JobId(const std::string &host, unsigned int port, const std::string &unique); - JobId(const std::string &host, const std::string &port, const std::string &unique); + explicit JobId(std::string const& host = std::string("localhost"), + int port = GLITE_JOBID_DEFAULT_PORT, + std::string const& unique = std::string("")); /** * Destructor. @@ -161,7 +91,7 @@ public: /** * Copy constructor. */ - JobId(const JobId&); + JobId(JobId const&); /** * Constructor from C jobid. @@ -173,7 +103,7 @@ public: * Assignment operator. * Create a deep copy of the JobId instance. */ - JobId& operator=(const JobId &src); + JobId& operator=(JobId const& src); /** * Casting operator. @@ -209,7 +139,7 @@ public: * Get port. * @return port */ - unsigned int port() const; + int port() const; /** * Get unique. @@ -219,13 +149,6 @@ public: //@} -protected: - - /** - * Clear all members. - */ - void clear(); - private: glite_jobid_t m_jobid; }; @@ -234,167 +157,162 @@ private: // -------------------- implementation ------------------------ -JobId::JobId() +inline +JobId::JobId(std::string const& job_id_string) { - int ret; - - ret = glite_jobid_create("localhost", GLITE_JOBID_DEFAULT_PORT, - &m_jobid); - if(ret) throw new Exception(__FILE__, __LINE__, __FUNCTION__, ret, - "Failed to create new jobid"); -} + int ret = glite_jobid_parse(job_id_string.c_str(), + &m_jobid); + switch(ret) { + case EINVAL: + throw JobIdError(job_id_string); + case ENOMEM: + throw std::bad_alloc; -JobId::JobId(const std::string &job_id_string) -{ - int ret; - - ret = glite_jobid_parse(job_id_string.c_str(), - &m_jobid); - if(ret) { - switch(ret) { - case EINVAL: - throw new Exception(__FILE__, __LINE__, __FUNCTION__, ret, - "Failed to parse jobid string"); - default: - throw new Exception(__FILE__, __LINE__, __FUNCTION__, ret, - "Failed to create new jobid"); - } + default: } } -JobId::JobId(const std::string &host, unsigned int port, const std::string &unique) +inline +JobId::JobId(std::string const& host, int port, std::string const& unique) { - int ret; - - ret = glite_jobid_create(host.c_str(), port, unique.size() ? unique.c_str() : NULL, - &m_jobid); - if(ret) throw new Exception(__FILE__, __LINE__, __FUNCTION__, ret, - "Failed to create new jobid"); -} + if(port < 0) { + throw JobIdError("negative port"); + } + int ret = glite_jobid_create(host.c_str(), port, + unique.empty() ? NULL : unique.c_str(), + &m_jobid); + switch(ret) { + case EINVAL: + throw JobIdError(host); + + case ENOMEM: + throw std::bad_alloc; -JobId::JobId(const std::string &host, const std::string &port, const std::string &unique) -{ - unsigned int i_port; - int ret; - istringstream is(port); - - i_port << is; - ret = glite_jobid_create(host.c_str(), i_port, unique.size() ? unique.c_str() : NULL, - &m_jobid); - if(ret) throw new Exception(__FILE__, __LINE__, __FUNCTION__, ret, - "Failed to create new jobid"); + default: + } } +inline JobId::~JobId() { - clear(); + glite_jobid_free(m_jobid); } -JobId::JobId(const JobId &src) +inline +JobId::JobId(JobId const& src) { - int ret; - - ret = glite_jobid_dup(src.m_jobid, - &m_jobid); - if(ret) throw new Exception(__FILE__, __LINE__, __FUNCTION__, ret, - "Failed to create new jobid"); + int ret = glite_jobid_dup(src.m_jobid, + &m_jobid); + if(ret) { + throw std::bad_alloc; + } } +inline JobId::JobId(glite_jobid_const_t src) { - int ret; + if(src == NULL) { + throw JobIdError("null"); + } - ret = glite_jobid_dup(src, - &m_jobid); - if(ret) throw new Exception(__FILE__, __LINE__, __FUNCTION__, ret, - "Failed to create new jobid"); + int ret = glite_jobid_dup(src, + &m_jobid); + if(ret) { + throw std::bad_alloc; + } } +inline JobId& - JobId::operator=(const JobId &src) +JobId::operator=(JobId const& src) { - int ret; - - clear(); - ret = glite_jobid_dup(src.m_jobid, - &m_jobid); - if(ret) throw new Exception(__FILE__, __LINE__, __FUNCTION__, ret, - "Failed to create new jobid"); - return(*this); + if(this == &src) { + return *this; + } + + glite_jobid_free(m_jobid); + int ret = glite_jobid_dup(src.m_jobid, + &m_jobid); + if(ret) { + throw std::bad_alloc; + } + return *this; } +inline glite_jobid_const_t - JobId::c_jobid() const +JobId::c_jobid() const { - return(m_jobid); + return m_jobid; } +inline std::string - JobId::toString() const +JobId::toString() const { - char *out; + char *out = glite_jobid_unparse(m_jobid); + std::string res(out); - out = glite_jobid_unparse(m_jobid); - return(std::string(out)); + free(out); + return res; } +inline std::string - JobId::server() const +JobId::server() const { - char *server; + char *server = glite_jobid_getServer(m_jobid); + std::string res(server); - server = glite_jobid_getServer(m_jobid); - return(std::string(server)); + free(server); + return res; } +inline std::string - JobId::host() const +JobId::host() const { char *name; unsigned int port; - glite_jobid_getServerParts(m_jobid, - &name, &port); - return(std::string(name)); + glite_jobid_getServerParts_internal(m_jobid, + &name, &port); + return std::string(name); } -unsigned int - JobId::port() const +inline +int +JobId::port() const { char *name; unsigned int port; - glite_jobid_getServerParts(m_jobid. - &name, &port); - return(port); + glite_jobid_getServerParts_internal(m_jobid. + &name, &port); + return port; } +inline std::string - JobId::unique() const -{ - char *unique; - - unique = glite_jobid_getUnique(m_jobid); - return(std::string(unique)); -} - - -void - JobId::clear() +JobId::unique() const { - glite_jobid_free(m_jobid); + char *unique = glite_jobid_getUnique_internal(m_jobid); + std::string res(unique); + + free(unique); + return res; } -- 1.8.2.3