From 69fafeb2656fa65c772f962430a8823cc304717f Mon Sep 17 00:00:00 2001 From: adelikat Date: Tue, 25 Jun 2024 10:57:14 -0500 Subject: [PATCH] PointsCalculator refactorings - make dto a record, remove unnecessary id prop, make some things internal, add more unit tests --- .../PointsService/PointsCalculator.cs | 17 ++---- .../Services/PointsService/PointsService.cs | 20 ++++--- .../Services/PointsCalculatorTests.cs | 52 ++++++++++--------- 3 files changed, 41 insertions(+), 48 deletions(-) diff --git a/TASVideos.Core/Services/PointsService/PointsCalculator.cs b/TASVideos.Core/Services/PointsService/PointsCalculator.cs index 08699a2d9..677411cf8 100644 --- a/TASVideos.Core/Services/PointsService/PointsCalculator.cs +++ b/TASVideos.Core/Services/PointsService/PointsCalculator.cs @@ -46,13 +46,15 @@ internal static double PlayerPointsForMovie(Publication publication, double aver var rawPoints = Math.Pow(Math.Max(publication.AverageRating ?? 0, 0), exp); var authorMultiplier = Math.Pow(publication.AuthorCount, -0.5); - var actual = rawPoints * authorMultiplier * publication.ClassWeight; + var actual = rawPoints * authorMultiplier * publication.Weight; if (actual < PlayerPointConstants.MinimumPlayerPointsForPublication) { actual = PlayerPointConstants.MinimumPlayerPointsForPublication; } + // We still factor in obsolete movies but only at a number less than zero + // for the purpose of determining "former player" rank if (publication.Obsolete) { actual *= PlayerPointConstants.ObsoleteMultiplier; @@ -80,16 +82,5 @@ internal static double RatingExponent(int total, double averageRatingPerPublicat /// /// Represents all the data necessary from a publication to factor into player points /// - public class Publication - { - public int Id { get; init; } - - // We still factor in obsolete movies but only at a number less than zero - // for the purpose of determining "former player" rank - public bool Obsolete { get; init; } - public double? AverageRating { get; init; } - public int RatingCount { get; init; } - public double ClassWeight { get; init; } - public int AuthorCount { get; init; } - } + internal record Publication(bool Obsolete, int RatingCount, int AuthorCount, double Weight, double? AverageRating); } diff --git a/TASVideos.Core/Services/PointsService/PointsService.cs b/TASVideos.Core/Services/PointsService/PointsService.cs index b49ea6ab3..c0bad8ce7 100644 --- a/TASVideos.Core/Services/PointsService/PointsService.cs +++ b/TASVideos.Core/Services/PointsService/PointsService.cs @@ -51,8 +51,9 @@ public async ValueTask PlayerPointsForPublication(int publicationId) } var publication = await db.Publications + .Where(p => p.Id == publicationId) .ToCalcPublication() - .SingleOrDefaultAsync(p => p.Id == publicationId); + .SingleOrDefaultAsync(); if (publication is null || publication.AuthorCount == 0) { @@ -90,17 +91,14 @@ internal static class PointsEntityExtensions { public static IQueryable ToCalcPublication(this IQueryable query) { - return query.Select(p => new PointsCalculator.Publication - { - Id = p.Id, - Obsolete = p.ObsoletedById.HasValue, - ClassWeight = p.PublicationClass!.Weight, - AuthorCount = p.Authors.Count, - RatingCount = p.PublicationRatings.Count, - AverageRating = p.PublicationRatings.Count > 0 ? p.PublicationRatings + return query.Select(p => new PointsCalculator.Publication( + p.ObsoletedById.HasValue, + p.PublicationRatings.Count, + p.Authors.Count, + p.PublicationClass!.Weight, + p.PublicationRatings.Count > 0 ? p.PublicationRatings .Where(pr => !pr.Publication!.Authors.Select(a => a.UserId).Contains(pr.UserId)) .Where(pr => pr.User!.UseRatings) - .Average(pr => pr.Value) : null - }); + .Average(pr => pr.Value) : null)); } } diff --git a/tests/TASVideos.Core.Tests/Services/PointsCalculatorTests.cs b/tests/TASVideos.Core.Tests/Services/PointsCalculatorTests.cs index 2f34d1851..3e4568c33 100644 --- a/tests/TASVideos.Core.Tests/Services/PointsCalculatorTests.cs +++ b/tests/TASVideos.Core.Tests/Services/PointsCalculatorTests.cs @@ -17,14 +17,7 @@ public void PlayerPoints_PublicationMinimum_IfNotObsolete() { var publications = new[] { - new PointsCalculator.Publication - { - Obsolete = false, - ClassWeight = 0, - RatingCount = 0, - AverageRating = 0, - AuthorCount = 1 - } + new PointsCalculator.Publication(false, 0, 1, 0, 0) }; var expected = publications.Length * PlayerPointConstants.MinimumPlayerPointsForPublication; @@ -37,14 +30,7 @@ public void PlayerPoints_BasicTest() { var publications = new[] { - new PointsCalculator.Publication - { - AverageRating = 4.45166667, - RatingCount = 6, - ClassWeight = 0.75, - Obsolete = false, - AuthorCount = 1 - } + new PointsCalculator.Publication(false, 6, 1, 0.75, 4.45166667) }; const double roundedExpected = 18.3; @@ -58,14 +44,7 @@ public void PlayerPoints_NegativeRating() { var publications = new[] { - new PointsCalculator.Publication - { - AverageRating = -100, - RatingCount = 6, - ClassWeight = 0.75, - Obsolete = false, - AuthorCount = 1 - } + new PointsCalculator.Publication(false, 6, 1, 0.75, -100) }; const int expected = PlayerPointConstants.MinimumPlayerPointsForPublication; @@ -73,6 +52,31 @@ public void PlayerPoints_NegativeRating() Assert.AreEqual(expected, actual); } + [TestMethod] + public void PlayerPoints_MultipleAuthors_ReducesPoints() + { + var publications = new[] + { + new PointsCalculator.Publication(false, 5, 2, 1, 5) + }; + + var actual = PointsCalculator.PlayerPoints(publications, AverageRatingsPerMovie); + + Assert.AreEqual(19.016, actual, 0.001); + } + + [TestMethod] + public void PlayerPointsForMovie_WithWeight_UsesWeight() + { + const double weightMultiplier = 2.0; + var publicationWithoutWeight = new PointsCalculator.Publication(false, 1, 1, 1, 5.0); + var withoutWeight = PointsCalculator.PlayerPointsForMovie(publicationWithoutWeight, 1); + + var publication = new PointsCalculator.Publication(false, 1, 1, weightMultiplier, 5.0); + var actual = PointsCalculator.PlayerPointsForMovie(publication, 1); + Assert.AreEqual(withoutWeight * weightMultiplier, actual, 0.001); + } + [TestMethod] [DataRow(-1, "")] [DataRow(0, "")]