Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

More ref args #504

Merged
merged 1 commit into from Feb 26, 2015
Merged

More ref args #504

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 8 additions & 8 deletions include/CoolProp.h
View file
Open in desktop
Expand Up @@ -63,10 +63,10 @@ You might want to start by looking at CoolProp.h

/// Set the global error string
/// @param error The error string to use
void set_error_string(std::string error);
void set_error_string(const std::string &error);
/// An internal function to set the global warning string
/// @param warning The string to set as the warning string
void set_warning_string(std::string warning);
void set_warning_string(const std::string &warning);

/* \brief Extract a value from the saturation ancillary
*
Expand All @@ -81,7 +81,7 @@ You might want to start by looking at CoolProp.h
/// Get a globally-defined string
/// @param ParamName A string, one of "version", "errstring", "warnstring", "gitrevision", "FluidsList", "fluids_list", "parameter_list","predefined_mixtures"
/// @returns str The string, or an error message if not valid input
std::string get_global_param_string(std::string ParamName);
std::string get_global_param_string(const std::string &ParamName);

/*/// Get a long that represents the fluid type
/// @param FluidName The fluid name as a string
Expand All @@ -103,7 +103,7 @@ You might want to start by looking at CoolProp.h

@returns The string, or an error message if not valid input
*/
std::string get_fluid_param_string(std::string FluidName, std::string ParamName);
std::string get_fluid_param_string(const std::string &FluidName, const std::string &ParamName);

/** \brief Check if the fluid name is valid
*
Expand All @@ -112,13 +112,13 @@ You might want to start by looking at CoolProp.h
* \note "gfreilgregre" -> false; "HEOS::Water" -> true; "Water" -> true
*
*/
bool is_valid_fluid_string(std::string &fluidstring);
bool is_valid_fluid_string(const std::string &fluidstring);

/// Returns the BibTeX key from the bibtex library of CoolProp corresponding to the item requested
/// @param FluidName The name of the fluid that is part of CoolProp, for instance "n-Propane"
/// @param item The key that is desired, one of "EOS", "CP0", "VISCOSITY", "CONDUCTIVITY", "ECS_LENNARD_JONES", "ECS_FITS", "SURFACE_TENSION"
/// @returns The BibTeX key
std::string get_BibTeXKey(std::string FluidName, std::string item);
std::string get_BibTeXKey(const std::string &FluidName, const std::string &item);

/**
\brief Set the reference state based on a string representation
Expand All @@ -140,15 +140,15 @@ You might want to start by looking at CoolProp.h
\f]
where \f$ \Delta s = s-s_{spec} \f$ and \f$ \Delta h = h-h_{spec} \f$
*/
void set_reference_stateS(std::string FluidName, std::string reference_state);
void set_reference_stateS(const std::string &FluidName, const std::string &reference_state);

/// Set the reference state based on a thermodynamic state point specified by temperature and molar density
/// @param FluidName The name of the fluid
/// @param T Temperature at reference state [K]
/// @param rhomolar Density at reference state [mol/m^3]
/// @param h0 Enthalpy at reference state [J/mol]
/// @param s0 Entropy at references state [J/mol/K]
void set_reference_stateD(std::string FluidName, double T, double rhomolar, double h0, double s0);
void set_reference_stateD(const std::string &FluidName, double T, double rhomolar, double h0, double s0);

/// Return a string representation of the phase
/// @param Name1 The first state variable name, one of "T","D","H",etc.
Expand Down
145 changes: 71 additions & 74 deletions include/CoolPropTools.h
View file
Open in desktop
Expand Up @@ -226,7 +226,7 @@
return str;
}

std::string strjoin(std::vector<std::string> strings, std::string delim);
std::string strjoin(const std::vector<std::string> &strings, const std::string &delim);

void MatInv_2(double A[2][2] , double B[2][2]);

Expand All @@ -245,7 +245,7 @@
{
return (y1-y0)/(x1-x0)*(x-x0)+y0;
};
template<class T> T LinearInterp(std::vector<T> x, std::vector<T> y, std::size_t i0, std::size_t i1, T val)
template<class T> T LinearInterp(const std::vector<T> &x, const std::vector<T> &y, std::size_t i0, std::size_t i1, T val)
{
return LinearInterp(x[i0],x[i1],y[i0],y[i1],val);
};
Expand All @@ -262,7 +262,7 @@
L2=((x-x0)*(x-x1))/((x2-x0)*(x2-x1));
return L0*f0+L1*f1+L2*f2;
};
template<class T> T QuadInterp(std::vector<T> x, std::vector<T> y, std::size_t i0, std::size_t i1, std::size_t i2, T val)
template<class T> T QuadInterp(const std::vector<T> &x, const std::vector<T> &y, std::size_t i0, std::size_t i1, std::size_t i2, T val)
{
return QuadInterp(x[i0],x[i1],x[i2],y[i0],y[i1],y[i2],val);
};
Expand All @@ -280,7 +280,7 @@
L3=((x-x0)*(x-x1)*(x-x2))/((x3-x0)*(x3-x1)*(x3-x2));
return L0*f0+L1*f1+L2*f2+L3*f3;
};
template<class T> T CubicInterp(std::vector<T> x, std::vector<T> y, std::size_t i0, std::size_t i1, std::size_t i2, std::size_t i3, T val)
template<class T> T CubicInterp(const std::vector<T> &x, const std::vector<T> &y, std::size_t i0, std::size_t i1, std::size_t i2, std::size_t i3, T val)
{
return CubicInterp(x[i0],x[i1],x[i2],x[i3],y[i0],y[i1],y[i2],y[i3],val);
};
Expand Down Expand Up @@ -312,7 +312,7 @@

inline bool double_equal(double a, double b){return std::abs(a - b) <= 1 * DBL_EPSILON * std::max(std::abs(a), std::abs(b));};

template<class T> T max_abs_value(std::vector<T> x)
template<class T> T max_abs_value(const std::vector<T> &x)
{
T max = 0;
std::size_t N = x.size();
Expand All @@ -324,7 +324,7 @@
return max;
}

template<class T> T min_abs_value(std::vector<T> x)
template<class T> T min_abs_value(const std::vector<T> &x)
{
T min = 1e40;
std::size_t N = x.size();
Expand All @@ -341,54 +341,62 @@
class Dictionary
{
private:
std::map<std::string, double> numbers;
std::map<std::string, std::string> strings;
std::map<std::string, std::vector<double> > double_vectors;
std::map<std::string, std::vector<std::string> > string_vectors;
typedef std::map<std::string, double> numbers_map;
numbers_map numbers;
typedef std::map<std::string, std::string> strings_map;
strings_map strings;
typedef std::map<std::string, std::vector<double> > double_vectors_map;
double_vectors_map double_vectors;
typedef std::map<std::string, std::vector<std::string> > string_vectors_map;
string_vectors_map string_vectors;
public:
Dictionary(){};
bool is_empty(void){return numbers.empty() && strings.empty() && double_vectors.empty() && string_vectors.empty();}
void add_string(std::string s1, std::string s2){ strings.insert(std::pair<std::string, std::string>(s1, s2));}
void add_number(std::string s1, double d){ numbers.insert(std::pair<std::string, double>(s1, d));}
void add_double_vector(std::string s1, std::vector<double> d){ double_vectors.insert(std::pair<std::string, std::vector<double> >(s1, d));}
void add_string_vector(std::string s1, std::vector<std::string> d){ string_vectors.insert(std::pair<std::string, std::vector<std::string> >(s1, d));}
std::string get_string(std::string s)
bool is_empty(void) const {return numbers.empty() && strings.empty() && double_vectors.empty() && string_vectors.empty();}
void add_string(const std::string &s1, const std::string &s2){ strings.insert(std::pair<std::string, std::string>(s1, s2));}
void add_number(const std::string &s1, double d){ numbers.insert(std::pair<std::string, double>(s1, d));}
void add_double_vector(const std::string &s1, const std::vector<double> &d){ double_vectors.insert(std::pair<std::string, std::vector<double> >(s1, d));}
void add_string_vector(const std::string &s1, const std::vector<std::string> &d){ string_vectors.insert(std::pair<std::string, std::vector<std::string> >(s1, d));}
std::string get_string(const std::string &s) const
{
if (strings.find(s) != strings.end()){
return strings[s];
}
else{
throw CoolProp::ValueError(format("%s could not be matched in get_string",s.c_str()));
strings_map::const_iterator i = strings.find(s);
if (i != strings.end()){
return i->second;
}
else{
throw CoolProp::ValueError(format("%s could not be matched in get_string",s.c_str()));
}
};
double get_double(std::string s)
double get_double(const std::string &s) const
{
if (numbers.find(s) != numbers.end()){
return numbers[s];
}
else{
throw CoolProp::ValueError(format("%s could not be matched in get_number",s.c_str()));
numbers_map::const_iterator i = numbers.find(s);
if (i != numbers.end()){
return i->second;
}
else{
throw CoolProp::ValueError(format("%s could not be matched in get_number",s.c_str()));
}
};
double get_number(std::string s)
double get_number(const std::string &s) const
{
return get_double(s);
};
const std::vector<double>& get_double_vector(std::string s)
const std::vector<double>& get_double_vector(const std::string &s) const
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't the const on the right not necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the rightmost const makes this function "constant function": it means that

  1. we declare that the function does not change the state of the object.
  2. thus, compiler does not allow us to change any of object's members, and only allows to call their "const" functions;

and only this allows to call this function, when the object itself is const.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another C++ lesson for me!

On Thu, Feb 26, 2015 at 1:54 PM, mikekaganski notifications@github.com
wrote:

In include/CoolPropTools.h
#504 (comment):

     {
         return get_double(s);
     };

  •    const std::vector<double>& get_double_vector(std::string s)

  •    const std::vector<double>& get_double_vector(const std::string &s) const

the rightmost const makes this function "constant function": it means that

  1. we declare that the function does not change the state of the object.
  2. thus, compiler does not allow us to change any of object's members, and
    only allows to call their "const" functions;

and only this allows to call this function, when the object itself is
const.

—
Reply to this email directly or view it on GitHub
https://github.com/CoolProp/CoolProp/pull/504/files#r25463186.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So there is an impact on the optimization possible since it is very const?

On Thu, Feb 26, 2015 at 1:54 PM, Ian Bell ian.h.bell@gmail.com wrote:

Another C++ lesson for me!

On Thu, Feb 26, 2015 at 1:54 PM, mikekaganski notifications@github.com
wrote:

In include/CoolPropTools.h
#504 (comment):

     {
         return get_double(s);
     };

  •    const std::vector<double>& get_double_vector(std::string s)

  •    const std::vector<double>& get_double_vector(const std::string &s) const

the rightmost const makes this function "constant function": it means that

  1. we declare that the function does not change the state of the object.
  2. thus, compiler does not allow us to change any of object's members,
    and only allows to call their "const" functions;

and only this allows to call this function, when the object itself is
const.

—
Reply to this email directly or view it on GitHub
https://github.com/CoolProp/CoolProp/pull/504/files#r25463186.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, and it was three-fold here.

  1. I noticed the param-by-value Dictionary (ReducingFunctions.h line 327). The dictionary has four maps, that contain numbers, strings, and vectors of numbers and strings. Each has to be recursively copied, and released after use, and each object has its constructors and destructors. So, naturally, I wanted to pass it by const ref. But when I did that, the error appeared that the object cannot be const, as the methods called are not const.
  2. I see that methods should not make changes to Dictionary - they are simple selectors. So I go and change their declarations as const, too. But compiler complains that as the method is const, it's illegal to call non-const methods of members there. I see, that the methods are implemented this way: first, the key is searched using find(), and only if found, the operator [] is called, that makes search again. This is not optimal per se, but additionally, in maps, operator [] is not const: it looks for the key, and if not found, it automatically adds new entry for this key, thus changing the object. This helped me to notice this inefficient implementation. So, I simplified things by making only one lookup using const method find().
  3. Further, it may help compiler generate more efficient code internally, even in places where the code is unchanged, as now it has a hint that the object's state stays unchanged.

{
if (double_vectors.find(s) != double_vectors.end()){
return double_vectors[s];
}
else{
double_vectors_map::const_iterator i = double_vectors.find(s);
if (i != double_vectors.end()){
return i->second;
}
else{
throw CoolProp::ValueError(format("%s could not be matched in get_double_vector",s.c_str()));
}
};
const std::vector<std::string>& get_string_vector(std::string s)
const std::vector<std::string>& get_string_vector(const std::string &s) const
{
if (string_vectors.find(s) != string_vectors.end()){
return string_vectors[s];
}
else{
string_vectors_map::const_iterator i = string_vectors.find(s);
if (i != string_vectors.end()){
return i->second;
}
else{
throw CoolProp::ValueError(format("%s could not be matched in get_string_vector",s.c_str()));
}
};
Expand Down Expand Up @@ -587,7 +595,7 @@ template<class T> void normalize_vector(std::vector<T> &x)
#endif
};

inline bool path_exists(std::string path)
inline bool path_exists(const std::string &path)
{
#if defined(__ISWINDOWS__) // Defined for 32-bit and 64-bit windows
struct _stat buf;
Expand Down Expand Up @@ -616,50 +624,39 @@ template<class T> void normalize_vector(std::vector<T> &x)
std::replace( file_path.begin(), file_path.end(), '\\', '/'); // replace all '\' with '/'

#if defined(__ISWINDOWS__)
char sep = '\\';
const char sep = '\\'; // well, Windows (and DOS) allows forward slash "/", too :)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But some functions in WINAPI do not accept forward slashes as the path delimiter. I'm not sure if that is the case within this function. I'd be happy to change this if we can show it works with /

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, I don't know which WinAPIs don't accept "/". It's better to know of them, so please give an example.
But either way, I don't feel like this should be changed, as it makes no difference efficiency-wise. This looks more robust, so I have no objections, only the notice.

#else
char sep = '/';
const char sep = '/';
#endif

std::vector<std::string> pathsplit = strsplit(file_path,'/');
std::string path = pathsplit[0];
if (pathsplit.size()>0)
std::string path = pathsplit[0]; // will throw if pathsplit.size() == 0
for (std::size_t i = 0, sz = pathsplit.size(); i < sz; i++)
{
for (unsigned int i = 0; i < pathsplit.size(); i++)
if (!path_exists(path))
{
if (!path_exists(path))
{
#if defined(__ISWINDOWS__) // Defined for 32-bit and 64-bit windows
#if defined(_UNICODE)
int errcode = CreateDirectoryA((LPCSTR)path.c_str(),NULL);
#else
int errcode = CreateDirectory((LPCSTR)path.c_str(),NULL);
#endif
if (errcode == 0){
switch(GetLastError()){
case ERROR_ALREADY_EXISTS:
break;
case ERROR_PATH_NOT_FOUND:
throw CoolProp::ValueError(format("Unable to make the directory %s",path.c_str()));
default:
break;
}

#if defined(__ISWINDOWS__) // Defined for 32-bit and 64-bit windows
int errcode = CreateDirectoryA((LPCSTR)path.c_str(),NULL);
if (errcode == 0){
switch(GetLastError()){
case ERROR_ALREADY_EXISTS:
break;
case ERROR_PATH_NOT_FOUND:
throw CoolProp::ValueError(format("Unable to make the directory %s",path.c_str()));
default:
break;
}
#else
#if defined(__powerpc__)
#else
mkdir(path.c_str(), S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH);
#endif

}
#else
#if defined(__powerpc__)
#else
mkdir(path.c_str(), S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH);
#endif
}
if (i < pathsplit.size()-1)
path += format("%c%s", sep, pathsplit[i+1].c_str());
#endif
}
}
else
{
throw CoolProp::ValueError(format("Could not make path [%s]",file_path.c_str()));
if (i < (sz-1))
path += format("%c%s", sep, pathsplit[i+1].c_str());
}
};
#endif
6 changes: 3 additions & 3 deletions include/Helmholtz.h
View file
Open in desktop
Expand Up @@ -783,7 +783,7 @@ class IdealHelmholtzPlanckEinsteinGeneralized : public BaseHelmholtzTerm{
public:
IdealHelmholtzPlanckEinsteinGeneralized(){N = 0; enabled = false;}
// Constructor with std::vector instances
IdealHelmholtzPlanckEinsteinGeneralized(std::vector<CoolPropDbl> n, std::vector<CoolPropDbl> theta, std::vector<CoolPropDbl> c, std::vector<CoolPropDbl> d)
IdealHelmholtzPlanckEinsteinGeneralized(const std::vector<CoolPropDbl> &n, const std::vector<CoolPropDbl> &theta, const std::vector<CoolPropDbl> &c, const std::vector<CoolPropDbl> &d)
:n(n), theta(theta), c(c), d(d)
{
N = n.size();
Expand All @@ -794,7 +794,7 @@ class IdealHelmholtzPlanckEinsteinGeneralized : public BaseHelmholtzTerm{
~IdealHelmholtzPlanckEinsteinGeneralized(){};

// Extend the vectors to allow for multiple instances feeding values to this function
void extend(std::vector<CoolPropDbl> n, std::vector<CoolPropDbl> theta, std::vector<CoolPropDbl> c, std::vector<CoolPropDbl> d)
void extend(const std::vector<CoolPropDbl> &n, const std::vector<CoolPropDbl> &theta, const std::vector<CoolPropDbl> &c, const std::vector<CoolPropDbl> &d)
{
this->n.insert(this->n.end(), n.begin(), n.end());
this->theta.insert(this->theta.end(), theta.begin(), theta.end());
Expand Down Expand Up @@ -992,7 +992,7 @@ class IdealHelmholtzCP0AlyLee : public BaseHelmholtzTerm{
IdealHelmholtzCP0AlyLee(){enabled = false;};

/// Constructor with std::vectors
IdealHelmholtzCP0AlyLee(std::vector<CoolPropDbl> c, double Tc, double T0)
IdealHelmholtzCP0AlyLee(const std::vector<CoolPropDbl> &c, double Tc, double T0)
:c(c), Tc(Tc), T0(T0)
{
tau0=Tc/T0;
Expand Down
6 changes: 3 additions & 3 deletions include/Solvers.h
View file
Open in desktop
Expand Up @@ -22,8 +22,8 @@ class FuncWrapperND
public:
FuncWrapperND(){};
virtual ~FuncWrapperND(){};
virtual std::vector<double> call(std::vector<double>) = 0;// must be provided
virtual std::vector<std::vector<double> > Jacobian(std::vector<double>);
virtual std::vector<double> call(const std::vector<double>&) = 0;// must be provided
virtual std::vector<std::vector<double> > Jacobian(const std::vector<double>&);
};

// Single-Dimensional solvers, pointer versions
Expand All @@ -40,7 +40,7 @@ double Newton(FuncWrapper1D &f, double x0, double ftol, int maxiter, std::string


// Multi-Dimensional solvers
std::vector<double> NDNewtonRaphson_Jacobian(FuncWrapperND *f, std::vector<double> x0, double tol, int maxiter, std::string *errstring);
std::vector<double> NDNewtonRaphson_Jacobian(FuncWrapperND *f, const std::vector<double> &x0, double tol, int maxiter, std::string *errstring);

}; /*namespace CoolProp*/
#endif
2 changes: 1 addition & 1 deletion src/Backends/Helmholtz/ExcessHEFunction.h
View file
Open in desktop
Expand Up @@ -125,7 +125,7 @@ class ExcessTerm
public:
std::size_t N;
std::vector<std::vector<DepartureFunctionPointer> > DepartureFunctionMatrix;
std::vector<std::vector<CoolPropDbl> > F;
STLMatrix F;

ExcessTerm(){};
~ExcessTerm(){};
Expand Down
2 changes: 1 addition & 1 deletion src/Backends/Helmholtz/MixtureParameters.cpp
View file
Open in desktop
Expand Up @@ -184,7 +184,7 @@ std::string get_mixture_binary_pair_data(const std::string &CAS1, const std::str
}
}

std::string get_reducing_function_name(std::string CAS1, std::string CAS2)
std::string get_reducing_function_name(const std::string &CAS1, const std::string &CAS2)
{
std::vector<std::string> CAS;
CAS.push_back(CAS1);
Expand Down