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.

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

IComparable in Vector2 and unit tests for it #47

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 29 additions & 1 deletion Hypercube.Math/Vectors/Vector2.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
namespace Hypercube.Math.Vectors;

[StructLayout(LayoutKind.Sequential)]
public readonly partial struct Vector2 : IEquatable<Vector2>
public readonly partial struct Vector2 : IEquatable<Vector2>, IComparable<Vector2>
{
public static readonly Vector2 NaN = new(float.NaN, float.NaN);
public static readonly Vector2 Zero = new(0, 0);
Expand Down Expand Up @@ -262,4 +262,32 @@ public static float Cross(Vector2 a, Vector2 b)
{
return a.X * b.Y - a.Y * b.X;
}

public int CompareTo(Vector2 other)
{
return Length.CompareTo(other.Length);
Copy link
Member

Choose a reason for hiding this comment

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

Касаемо этого, я думал насчёт использование чистой длины, но как мне известно для большого количество итераций, взятие корня будет вызывать лаги (А мы ведь используем вектора в физике и отрисовке). Суть в том, что лучше будет использовать LengthSqueared

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Действительно, взятие корня будет накладно по производительности. Исправлю на сравнение по квадратам площади

}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public int CompareTo(Vector2 other, ComparisonType comparisonType)
{
switch (comparisonType)
{
case ComparisonType.XComponent:
return X.CompareTo(other.X);
case ComparisonType.YComponent:
return Y.CompareTo(other.Y);
case ComparisonType.Angle:
return Angle.CompareTo(other.Angle);
default:
throw new ArgumentOutOfRangeException(nameof(comparisonType), comparisonType, "Invalid ComparisonType");
}
}

public enum ComparisonType
Copy link
Member

Choose a reason for hiding this comment

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

Думаю, это может использоваться для Vector2Int или Vector3 т.к. по именованию он не привязан Vector2, тогда нам его нужно вынести. Но данная практика мне кажется немного странной. По сколько мы и так можем легко сравнить vector.X > a, это не возымеет трудностей, но вот enum вносит непорядок в нашу реализацию, если интересно, на эту тему можно подискутировать.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Я думаю, что можно оставить этот enum в коде, но переместить его в более подходящее место.

У такого подхода есть свои плюсы:

  1. Впоследствии этот enum может быть полезен и в других классах (как вы и написали в Vector2Int или Vector3 )
  2. Реализация без этого enum выглядит достаточно топорно и не очень красиво. В данном случае можно создать отдельный метод под каждый из вариантов сравнения,
    либо сделать тоже самое, но оставить CompareTo для сравнения по длине, и последний вариант - сделать нечто подобное:
public int CompareTo<T>(T other, Func<Vector2, T, int> comparisonFunction)
{
    return comparisonFunction(this, other);
}

Copy link
Member

Choose a reason for hiding this comment

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

Мне все же кажется что лаконичнее вызывать vector.X.CompareTo(vlaue); чем использовать большой енам

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Мне все же кажется что лаконичнее вызывать vector.X.CompareTo(vlaue); чем использовать большой енам

Добавил реализацию, надеюсь верно Вас понял

{
XComponent,
YComponent,
Angle
}
}
111 changes: 111 additions & 0 deletions Hypercube.UnitTests/Math/CompareToTest.cs
Copy link
Member

Choose a reason for hiding this comment

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

На будущее, я пока не написал гайдлайны, если есть возможность лучше использовать var, вместо прямого указание типа, хоть это и не сходится с нотацией от Microsoft

Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
using System;
using System.Collections.Generic;
using System.Linq;
using Hypercube.Math.Vectors;
using System.Text;
using System.Threading.Tasks;
using static Hypercube.Math.Vectors.Vector2;

namespace Hypercube.UnitTests.Math;

[TestFixture]
public class CompareToTest
Copy link
Member

Choose a reason for hiding this comment

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

Классы, которые мы не предполагаем, что будут наследованы (Когда либо). Мы помечаем как sealed

{
[Test]
public void CompareTo_LengthComparison_ReturnsExpectedResult()
{
// Arrange
Vector2 vector1 = new Vector2(3, 4); // Length = 5
Vector2 vector2 = new Vector2(6, 8); // Length = 10

// Act
int result = vector1.CompareTo(vector2);

// Assert
Assert.Less(result, 0);
}

[Test]
public void CompareTo_XComponentComparison_ReturnsExpectedResult()
{
// Arrange
Vector2 vector1 = new Vector2(3, 4);
Vector2 vector2 = new Vector2(6, 4);

// Act
int result = vector1.CompareTo(vector2, ComparisonType.XComponent);

// Assert
Assert.Less(result, 0); // Expecting vector1 to be "less than" vector2 based on X component
}

[Test]
public void CompareTo_YComponentComparison_ReturnsExpectedResult()
{
// Arrange
Vector2 vector1 = new Vector2(3, 4);
Vector2 vector2 = new Vector2(3, 2);

// Act
int result = vector1.CompareTo(vector2, ComparisonType.YComponent);

// Assert
Assert.Greater(result, 0); // Expecting vector1 to be "greater than" vector2 based on Y component
}

[Test]
public void CompareTo_AngleComparison_ReturnsExpectedResult()
{
// Arrange
Vector2 vector1 = new Vector2(0, 1); // Angle = π/2 (90 degrees)
Vector2 vector2 = new Vector2(1, 0); // Angle = 0 degrees

// Act
int result = vector1.CompareTo(vector2, ComparisonType.Angle);

// Assert
Assert.Greater(result, 0); // Expecting vector1 to be "greater than" vector2 based on Angle
}

[Test]
public void CompareTo_SameVectors_ReturnsZero()
{
// Arrange
Vector2 vector1 = new Vector2(3, 4);
Vector2 vector2 = new Vector2(3, 4);

// Act & Assert
Assert.AreEqual(0, vector1.CompareTo(vector2));

Check warning on line 78 in Hypercube.UnitTests/Math/CompareToTest.cs

View workflow job for this annotation

GitHub Actions / Windows Build

Consider using the constraint model, Assert.That(actual, Is.EqualTo(expected)), instead of the classic model, Assert.AreEqual(expected, actual) (https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit2005.md)

Check warning on line 78 in Hypercube.UnitTests/Math/CompareToTest.cs

View workflow job for this annotation

GitHub Actions / Windows Build

Consider using the constraint model, Assert.That(actual, Is.EqualTo(expected)), instead of the classic model, Assert.AreEqual(expected, actual) (https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit2005.md)
Assert.AreEqual(0, vector1.CompareTo(vector2, ComparisonType.XComponent));

Check warning on line 79 in Hypercube.UnitTests/Math/CompareToTest.cs

View workflow job for this annotation

GitHub Actions / Windows Build

Consider using the constraint model, Assert.That(actual, Is.EqualTo(expected)), instead of the classic model, Assert.AreEqual(expected, actual) (https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit2005.md)

Check warning on line 79 in Hypercube.UnitTests/Math/CompareToTest.cs

View workflow job for this annotation

GitHub Actions / Windows Build

Consider using the constraint model, Assert.That(actual, Is.EqualTo(expected)), instead of the classic model, Assert.AreEqual(expected, actual) (https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit2005.md)
Assert.AreEqual(0, vector1.CompareTo(vector2, ComparisonType.YComponent));

Check warning on line 80 in Hypercube.UnitTests/Math/CompareToTest.cs

View workflow job for this annotation

GitHub Actions / Windows Build

Consider using the constraint model, Assert.That(actual, Is.EqualTo(expected)), instead of the classic model, Assert.AreEqual(expected, actual) (https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit2005.md)

Check warning on line 80 in Hypercube.UnitTests/Math/CompareToTest.cs

View workflow job for this annotation

GitHub Actions / Windows Build

Consider using the constraint model, Assert.That(actual, Is.EqualTo(expected)), instead of the classic model, Assert.AreEqual(expected, actual) (https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit2005.md)
Assert.AreEqual(0, vector1.CompareTo(vector2, ComparisonType.Angle));

Check warning on line 81 in Hypercube.UnitTests/Math/CompareToTest.cs

View workflow job for this annotation

GitHub Actions / Windows Build

Consider using the constraint model, Assert.That(actual, Is.EqualTo(expected)), instead of the classic model, Assert.AreEqual(expected, actual) (https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit2005.md)
}

[Test]
public void CompareTo_DifferentComparisons_ReturnDifferentResults()
{
// Arrange
Vector2 vector1 = new Vector2(1, 2);
Vector2 vector2 = new Vector2(2, 1);

// Act
int lengthComparison = vector1.CompareTo(vector2);
int xComponentComparison = vector1.CompareTo(vector2, ComparisonType.XComponent);
int yComponentComparison = vector1.CompareTo(vector2, ComparisonType.YComponent);

// Assert
Assert.AreNotEqual(lengthComparison, xComponentComparison);

Check warning on line 97 in Hypercube.UnitTests/Math/CompareToTest.cs

View workflow job for this annotation

GitHub Actions / Windows Build

Consider using the constraint model, Assert.That(actual, Is.Not.EqualTo(expected)), instead of the classic model, Assert.AreNotEqual(expected, actual) (https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit2006.md)

Check warning on line 97 in Hypercube.UnitTests/Math/CompareToTest.cs

View workflow job for this annotation

GitHub Actions / Windows Build

Consider using the constraint model, Assert.That(actual, Is.Not.EqualTo(expected)), instead of the classic model, Assert.AreNotEqual(expected, actual) (https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit2006.md)
Assert.AreNotEqual(xComponentComparison, yComponentComparison);

Check warning on line 98 in Hypercube.UnitTests/Math/CompareToTest.cs

View workflow job for this annotation

GitHub Actions / Windows Build

Consider using the constraint model, Assert.That(actual, Is.Not.EqualTo(expected)), instead of the classic model, Assert.AreNotEqual(expected, actual) (https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit2006.md)

Check warning on line 98 in Hypercube.UnitTests/Math/CompareToTest.cs

View workflow job for this annotation

GitHub Actions / Windows Build

Consider using the constraint model, Assert.That(actual, Is.Not.EqualTo(expected)), instead of the classic model, Assert.AreNotEqual(expected, actual) (https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit2006.md)
}

[Test]
public void CompareTo_InvalidComparisonType_ThrowsArgumentOutOfRangeException()
{
// Arrange
Vector2 vector1 = new Vector2(3, 4);
Vector2 vector2 = new Vector2(3, 4);

// Act & Assert
Assert.Throws<ArgumentOutOfRangeException>(() => vector1.CompareTo(vector2, (ComparisonType)999));
}
}
Loading