Skip to content

Commit

Permalink
Addressed several code smells related to custom processes
Browse files Browse the repository at this point in the history
Changes include:
- Made several `protected` data members `private`. Added getter functions to access them when needed.
- Replaced some explicit type names by `auto`.
- Declared some member functions `const`.
- Changed a parameter type to a reference-to-const.
- Removed an overridden member function since its body was identical to the base class implementation.
- Use a range-based for loop rather than an index to iterate a vector.
- Added a `static_cast` to avoid a difference in signedness.
- Initialized a pointer when it is declared. Also moved the declaration closer to its first usage.
- Reformatted most changes using Clang-format.
- Rephrased a comment.
- Renamed several variables/function parameters. These were not compliant with the Kratos Style Guide.
  • Loading branch information
avdg81 authored Mar 15, 2024
1 parent bfbef23 commit 358979a
Show file tree
Hide file tree
Showing 5 changed files with 181 additions and 137 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,14 @@ class ApplyBoundaryHydrostaticPressureTableProcess : public ApplyConstantBoundar
{
KRATOS_TRY

const Variable<double> &var = KratosComponents< Variable<double> >::Get(mVariableName);
const double Time = mrModelPart.GetProcessInfo()[TIME]/mTimeUnitConverter;
const double deltaH = mpTable->GetValue(Time);

block_for_each(mrModelPart.Nodes(), [&deltaH, &var, this](Node& rNode){
const double distance = mReferenceCoordinate - rNode.Coordinates()[mGravityDirection];
const double pressure = mSpecificWeight * (distance + deltaH);
rNode.FastGetSolutionStepValue(var) = std::max(pressure,0.0);
const auto& r_variable = KratosComponents<Variable<double>>::Get(GetVariableName());
const auto time = GetModelPart().GetProcessInfo()[TIME] / mTimeUnitConverter;
const auto delta_h = mpTable->GetValue(time);

block_for_each(GetModelPart().Nodes(), [&delta_h, &r_variable, this](Node& rNode) {
const auto distance = GetReferenceCoordinate() - rNode.Coordinates()[GetGravityDirection()];
const auto pressure = GetSpecificWeight() * (distance + delta_h);
rNode.FastGetSolutionStepValue(r_variable) = std::max(pressure, 0.0);
});
KRATOS_CATCH("")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,18 +52,20 @@ class ApplyBoundaryPhreaticLinePressureTableProcess : public ApplyConstantBounda
{
KRATOS_TRY

const Variable<double> &var = KratosComponents< Variable<double> >::Get(mVariableName);
const double Time = mrModelPart.GetProcessInfo()[TIME]/mTimeUnitConverter;
const double deltaH = mpTable->GetValue(Time);

block_for_each(mrModelPart.Nodes(), [&deltaH, &var, this](Node& rNode){
double xcoord = rNode.Coordinates()[mHorizontalDirection];
xcoord = std::max(xcoord,mMinHorizontalCoordinate);
xcoord = std::min(xcoord,mMaxHorizontalCoordinate);
double height = mSlope * ( xcoord - mFirstReferenceCoordinate[mHorizontalDirection]) + mFirstReferenceCoordinate[mGravityDirection];
const double distance = height - rNode.Coordinates()[mGravityDirection];
const double pressure = mSpecificWeight * (distance + deltaH);
rNode.FastGetSolutionStepValue(var) = std::max(pressure,0.0);
const auto& r_variable = KratosComponents<Variable<double>>::Get(GetVariableName());
const auto time = GetModelPart().GetProcessInfo()[TIME] / mTimeUnitConverter;
const auto delta_h = mpTable->GetValue(time);

block_for_each(GetModelPart().Nodes(), [&delta_h, &r_variable, this](Node& rNode) {
auto xcoord = rNode.Coordinates()[GetHorizontalDirection()];
xcoord = std::max(xcoord, GetMinHorizontalCoordinate());
xcoord = std::min(xcoord, GetMaxHorizontalCoordinate());
const auto height =
GetSlope() * (xcoord - GetFirstReferenceCoordinate()[GetHorizontalDirection()]) +
GetFirstReferenceCoordinate()[GetGravityDirection()];
const auto distance = height - rNode.Coordinates()[GetGravityDirection()];
const auto pressure = GetSpecificWeight() * (distance + delta_h);
rNode.FastGetSolutionStepValue(r_variable) = std::max(pressure, 0.0);
});

KRATOS_CATCH("")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ class ApplyConstantBoundaryHydrostaticPressureProcess : public Process
"table" : 1
} )" );

// Some values need to be mandatorily prescribed since no meaningful default value exist. For this reason try accessing to them
// So that an error is thrown if they don't exist
// Some values have to be input by the user since no meaningful default value exist. For
// this reason, we try to access them, so that an error is thrown if they don't exist.
rParameters["reference_coordinate"];
rParameters["variable_name"];
rParameters["model_part_name"];
Expand All @@ -59,7 +59,7 @@ class ApplyConstantBoundaryHydrostaticPressureProcess : public Process

mVariableName = rParameters["variable_name"].GetString();
mIsFixed = rParameters["is_fixed"].GetBool();
mGravityDirection = rParameters["gravity_direction"].GetInt();
mGravityDirection = static_cast<unsigned int>(rParameters["gravity_direction"].GetInt());
mReferenceCoordinate = rParameters["reference_coordinate"].GetDouble();
mSpecificWeight = rParameters["specific_weight"].GetDouble();

Expand All @@ -77,14 +77,15 @@ class ApplyConstantBoundaryHydrostaticPressureProcess : public Process
{
KRATOS_TRY

const Variable<double> &var = KratosComponents< Variable<double> >::Get(mVariableName);
const auto& r_variable = KratosComponents<Variable<double>>::Get(GetVariableName());

block_for_each(mrModelPart.Nodes(), [&var, this](Node& rNode) {
if (mIsFixed) rNode.Fix(var);
else if (mIsFixedProvided) rNode.Free(var);
block_for_each(GetModelPart().Nodes(), [&r_variable, this](Node& rNode) {
if (mIsFixed) rNode.Fix(r_variable);
else if (mIsFixedProvided) rNode.Free(r_variable);

const double pressure = mSpecificWeight * (mReferenceCoordinate - rNode.Coordinates()[mGravityDirection]);
rNode.FastGetSolutionStepValue(var) = std::max(pressure, 0.);
const auto pressure = GetSpecificWeight() * (GetReferenceCoordinate() -
rNode.Coordinates()[GetGravityDirection()]);
rNode.FastGetSolutionStepValue(r_variable) = std::max(pressure, 0.);
});

KRATOS_CATCH("")
Expand All @@ -96,15 +97,24 @@ class ApplyConstantBoundaryHydrostaticPressureProcess : public Process
return "ApplyConstantBoundaryHydrostaticPressureProcess";
}

protected:
/// Member Variables
ModelPart& mrModelPart;
std::string mVariableName;
bool mIsFixed;
bool mIsFixedProvided;
ModelPart& GetModelPart() { return mrModelPart; }

[[nodiscard]] const std::string& GetVariableName() const { return mVariableName; }

[[nodiscard]] unsigned int GetGravityDirection() const { return mGravityDirection; }

[[nodiscard]] double GetReferenceCoordinate() const { return mReferenceCoordinate; }

[[nodiscard]] double GetSpecificWeight() const { return mSpecificWeight; }

private:
ModelPart& mrModelPart;
std::string mVariableName;
bool mIsFixed;
bool mIsFixedProvided;
unsigned int mGravityDirection;
double mReferenceCoordinate;
double mSpecificWeight;
double mReferenceCoordinate;
double mSpecificWeight;
};

}
Original file line number Diff line number Diff line change
Expand Up @@ -129,21 +129,43 @@ class ApplyConstantBoundaryPhreaticLinePressureProcess : public Process
}

protected:
/// Member Variables
ModelPart& mrModelPart;
std::string mVariableName;
bool mIsFixed;
bool mIsFixedProvided;
[[nodiscard]] const ModelPart& GetModelPart() const { return mrModelPart; }

[[nodiscard]] ModelPart& GetModelPart() { return mrModelPart; }

[[nodiscard]] const std::string& GetVariableName() const { return mVariableName; }

[[nodiscard]] unsigned int GetGravityDirection() const { return mGravityDirection; }

[[nodiscard]] unsigned int GetHorizontalDirection() const { return mHorizontalDirection; }

[[nodiscard]] double GetSpecificWeight() const { return mSpecificWeight; }

[[nodiscard]] const Vector3& GetFirstReferenceCoordinate() const
{
return mFirstReferenceCoordinate;
}

[[nodiscard]] double GetSlope() const { return mSlope; }

[[nodiscard]] double GetMinHorizontalCoordinate() const { return mMinHorizontalCoordinate; }

[[nodiscard]] double GetMaxHorizontalCoordinate() const { return mMaxHorizontalCoordinate; }

private:
ModelPart& mrModelPart;
std::string mVariableName;
bool mIsFixed;
bool mIsFixedProvided;
unsigned int mGravityDirection;
unsigned int mHorizontalDirection;
double mSpecificWeight;
double mSpecificWeight;
unsigned int mOutOfPlaneDirection;
Vector3 mFirstReferenceCoordinate;
Vector3 mSecondReferenceCoordinate;
double mSlope;
double mMinHorizontalCoordinate;
double mMaxHorizontalCoordinate;

Vector3 mFirstReferenceCoordinate;
Vector3 mSecondReferenceCoordinate;
double mSlope;
double mMinHorizontalCoordinate;
double mMaxHorizontalCoordinate;
};

}
Loading

0 comments on commit 358979a

Please sign in to comment.