Skip to content

Instantly share code, notes, and snippets.

@Cygon
Last active November 11, 2017 17:50
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save Cygon/2694d9f72294d75ba77a611b9407ab10 to your computer and use it in GitHub Desktop.
Save Cygon/2694d9f72294d75ba77a611b9407ab10 to your computer and use it in GitHub Desktop.
Prefer Static Methods for Non-Mutating Operations

Prefer Static Methods for Non-Mutating Operations

When writing instance methods that do not change the state of their instance but rather return a modified copy of the instance, it is often better to implement these methods as static methods accepting an instance as an argument rather as a argument-less instance method.

DO NOT Return Mutated State from Instance Methods

Often, implementors decide to use instance methods to return a clone of the object with mutated state:

class Vector2 {
  
  public: Vector2 Normalize() const {
    float squaredLength = (this->X * this->X) + (this->Y * this->Y)
    if(squaredLength >= std::numeric_limit<float>::epsilon()) {
      float length = Math::SquareRoot(squaredLength);
      return Vector2(this->X / length, this->Y / length);
    } else {
      return Vector2::Zero;
    }
  }
  
  private: float X;
  private: float Y;
  
};

int main() {
  Vector2 direction = getEnemyPosition() - getOwnPosition();
  direction.Normalize(); // Unclear; does nothing!
}

This can be very confusing for users of these methods, as it is not immediately clear that the instance itself remains unchanged.

If the above method was renamed to 'GetNormalized()', its usage on intermediate results would still result in unconventional syntax:

int main() {
  Vector2 direction = (getEnemyPosition() - getOwnPosition()).Normalize();
  direction.Normalize(); // Unclear; does nothing!
}

Finally, naming it like an accessor method will dilute the fact that the result is being calculated and not merely accessed.

DO NOT Write Instance Methods when Instance has Same Rank as Argument

Another typical case is an operation with two arguments where both arguments are simply inputs to an operation:

class Vector3 {
  
  public: Vector3 Cross(const Vector3 &other) const {
    return Vector3(
      this->Y * other.Z - this->Z * other.Y,
      this->Z * other.X - this->X * other.Z,
      this->X * other.Y - this->Y * other.X);        
    );
  }
  
  private: float X;
  private: float Y;
  private: float Z;
  
};

int main() {
  Vector3 forward = getForwardVector();
  Vector3 right = forward.Cross(Vector3::Up);
}

For a vector cross product, like in the above example, is one operand somehow the source of the operation and doing something to the other?

No, both a operands to the cross product operation are ranked equally. This is better represented with a static method where both operands are passed as parameters.

DO Use Static Methods if Both Arguments are of Equal Rank

Using static methods in this case will greatly improve readability of operations with equally-ranked parameters.

class Vector3 {
  
  public: static Vector3 Cross(const Vector3 &left, const Vector3 &right) {
    return Vector3(
      left.Y * other.Z - left.Z * other.Y,
      left.Z * other.X - left.X * other.Z,
      left.X * other.Y - left.Y * other.X);        
    );
  }
  
  private: float X;
  private: float Y;
  private: float Z;
  
};

int main() {
  Vector3 forward = getForwardVector();
  Vector3 right = Vector3::Cross(forward, Vector3::Up);
}

It will also help avoid user error in cases where the original instance is not being modified:

class Vector2 {
  
  public: static Vector2 Normalize(Vector2 vector) {
    float squaredLength = (vector.X * vector.X) + (vector.Y * vector.Y)
    if(squaredLength >= std::numeric_limit<float>::epsilon()) {
      float length = Math::SquareRoot(squaredLength);
      return Vector2(vector.X / length, vector.Y / length);
    } else {
      return Vector2::Zero;
    }
  }
  
  private: float X;
  private: float Y;
  
};

int main() {
  Vector2 direction = Vector2::Normalize(getEnemyPosition() - getOwnPosition());
}

In both code snippets, it is now clear that a vector operation is being performed.

Passing intermediate results as parameters will not result in a method being called on an unknown, intermediate object of unknown type since now the Vector::Method() syntax clearly structures and reveals the type and purpose ofthe intermediate values.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment