From 2ddef25177fd0744558bffa82597d29d446f85ee Mon Sep 17 00:00:00 2001 From: spiral Date: Fri, 27 Aug 2021 19:18:59 -0400 Subject: [PATCH] refactor: don't use a class for GroupAddRemoveResponse; fix tests --- .../Services/GroupAddRemoveResponseService.cs | 101 ++++++------------ .../GroupAddRemoveResponseTests.cs | 2 +- 2 files changed, 36 insertions(+), 67 deletions(-) diff --git a/PluralKit.Bot/Services/GroupAddRemoveResponseService.cs b/PluralKit.Bot/Services/GroupAddRemoveResponseService.cs index be91d5f0..75049fd7 100644 --- a/PluralKit.Bot/Services/GroupAddRemoveResponseService.cs +++ b/PluralKit.Bot/Services/GroupAddRemoveResponseService.cs @@ -7,103 +7,72 @@ namespace PluralKit.Bot public static class GroupAddRemoveResponseService { public static string GenerateResponse(Groups.AddRemoveOperation action, int memberCount, int groupCount, int actionedOn, int notActionedOn) - => new Response(action, memberCount, groupCount, actionedOn, notActionedOn).ToString(); - - private class Response { - private readonly Groups.AddRemoveOperation _op; + var op = action; - private readonly string _actionStr; - private readonly string _containStr; - private readonly string _emojiStr; + var actionStr = action == Groups.AddRemoveOperation.Add ? "added to" : "removed from"; + var containStr = action == Groups.AddRemoveOperation.Add ? "in" : "not in"; + var emojiStr = actionedOn > 0 ? Emojis.Success : Emojis.Error; - private readonly bool _memberPlural; - private readonly bool _groupPlural; + var memberPlural = memberCount > 1; + var groupPlural = groupCount > 1; - private readonly int _actionedOn; - private readonly int _notActionedOn; + // sanity checking: we can't add multiple groups to multiple members (at least for now) + if (memberPlural && groupPlural) + throw new ArgumentOutOfRangeException(); - public Response(Groups.AddRemoveOperation action, int memberCount, int groupCount, int actionedOn, - int notActionedOn) - { - _op = action; - - _actionStr = action == Groups.AddRemoveOperation.Add ? "added to" : "removed from"; - _containStr = action == Groups.AddRemoveOperation.Add ? "in" : "not in"; - _emojiStr = actionedOn > 0 ? Emojis.Success : Emojis.Error; - - _memberPlural = memberCount > 1; - _groupPlural = groupCount > 1; - - // sanity checking: we can't add multiple groups to multiple members (at least for now) - if (_memberPlural && _groupPlural) - throw new ArgumentOutOfRangeException(); - - // sanity checking: we can't act/not act on a different number of entities than we have - if (_memberPlural && (actionedOn + notActionedOn) != memberCount) - throw new ArgumentOutOfRangeException(); - if (_groupPlural && (actionedOn + notActionedOn) != groupCount) - throw new ArgumentOutOfRangeException(); - - _actionedOn = actionedOn; - _notActionedOn = notActionedOn; - } + // sanity checking: we can't act/not act on a different number of entities than we have + if (memberPlural && (actionedOn + notActionedOn) != memberCount) + throw new ArgumentOutOfRangeException(); + if (groupPlural && (actionedOn + notActionedOn) != groupCount) + throw new ArgumentOutOfRangeException(); // name generators - private string MemberString(bool capitalize = false) - => capitalize - ? (_memberPlural ? "Members" : "Member") - : (_memberPlural ? "members" : "member"); - - private string MemberString(int count, bool capitalize = false) + string MemberString(int count, bool capitalize = false) => capitalize ? (count == 1 ? "Member" : "Members") : (count == 1 ? "member" : "members"); - private string GroupString() => _groupPlural ? "groups" : "group"; - - private string GroupString(int count) + string GroupString(int count) => count == 1 ? "group" : "groups"; // string generators - private string ResponseString() + string ResponseString() { - if (_actionedOn > 0 && _notActionedOn > 0 && _memberPlural) - return $"{_actionedOn} {MemberString(_actionedOn)} {_actionStr} {GroupString()}"; - if (_actionedOn > 0 && _notActionedOn > 0 && _groupPlural) - return $"{MemberString(capitalize: true)} {_actionStr} {_actionedOn} {GroupString(_actionedOn)}"; - if (_notActionedOn == 0) - return $"{MemberString(capitalize: true)} {_actionStr} {GroupString()}"; - if (_actionedOn == 0) - return $"{MemberString(capitalize: true)} not {_actionStr} {GroupString()}"; + if (actionedOn > 0 && notActionedOn > 0 && memberPlural) + return $"{actionedOn} {MemberString(actionedOn)} {actionStr} {GroupString(groupCount)}"; + if (actionedOn > 0 && notActionedOn > 0 && groupPlural) + return $"{MemberString(memberCount, capitalize: true)} {actionStr} {actionedOn} {GroupString(actionedOn)}"; + if (notActionedOn == 0) + return $"{MemberString(memberCount, capitalize: true)} {actionStr} {GroupString(groupCount)}"; + if (actionedOn == 0) + return $"{MemberString(memberCount, capitalize: true)} not {actionStr} {GroupString(groupCount)}"; throw new ArgumentOutOfRangeException(); } - private string InfoMessage() + string InfoMessage() { - if (_notActionedOn == 0) return $""; + if (notActionedOn == 0) return $""; var msg = ""; - if (_actionedOn > 0 && _memberPlural) - msg += $"{_notActionedOn} {MemberString(_notActionedOn)}"; + if (actionedOn > 0 && memberPlural) + msg += $"{notActionedOn} {MemberString(notActionedOn)}"; else - msg += $"{MemberString()}"; + msg += $"{MemberString(memberCount)}"; - msg += $" already {_containStr}"; + msg += $" already {containStr}"; - if (_actionedOn > 0 && _groupPlural) - msg += $" {_notActionedOn} {GroupString(_notActionedOn)}"; + if (actionedOn > 0 && groupPlural) + msg += $" {notActionedOn} {GroupString(notActionedOn)}"; else - msg += $" {GroupString()}"; + msg += $" {GroupString(groupCount)}"; return $" ({msg})"; } - public string ToString() => $"{_emojiStr} {ResponseString()}{InfoMessage()}."; - - // | + return $"{emojiStr} {ResponseString()}{InfoMessage()}."; } } } \ No newline at end of file diff --git a/PluralKit.Tests/GroupAddRemoveResponseTests.cs b/PluralKit.Tests/GroupAddRemoveResponseTests.cs index 3b56f345..0d91633a 100644 --- a/PluralKit.Tests/GroupAddRemoveResponseTests.cs +++ b/PluralKit.Tests/GroupAddRemoveResponseTests.cs @@ -158,7 +158,7 @@ namespace PluralKit.Tests func(removeOp, 2, 1, 2, 0) ); - + [Fact] public void PartialSuccess1() => Assert.Equal( $"{success} 1 member removed from group (2 members already not in group).",