Add BanWordParser and related unit tests #55
Conversation
Introduces BanWordParser for parsing ban words and UGC ban words from XML files. Adds BanWordParserTest to verify correct parsing and updates project version to 2.3.4.
WalkthroughIntroduces BanWordParser to extract ban words from XML via M2dReader, adds two parsing methods (banword and ugcbanword), includes corresponding unit tests validating counts and non-empty names, and bumps Maple2.File.Parser package version from 2.3.3 to 2.3.4. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Test as Test
participant Parser as BanWordParser
participant Reader as M2dReader
participant Xml as XmlSerializer
Test->>Parser: new BanWordParser(M2dReader)
Test->>Parser: ParseBanWords()
loop For each entry name containing "banword" and not "ugc"
Parser->>Reader: Open(entry)
Reader-->>Parser: XML stream
Parser->>Xml: Deserialize<StringMapping>(stream)
Xml-->>Parser: StringMapping
alt Non-empty values
Parser-->>Test: yield (Id, Name)
end
end
Test->>Parser: ParseUgcBanWords()
loop For each entry name containing "ugcbanword"
Parser->>Reader: Open(entry)
Reader-->>Parser: XML stream
Parser->>Xml: Deserialize<StringMapping>(stream)
Xml-->>Parser: StringMapping
alt Non-empty values
Parser-->>Test: yield (Id, Name)
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
Maple2.File.Tests/BanWordParserTest.cs (1)
11-23: Consider extracting common setup to a test initialization method.Both test methods duplicate the Filter.Load call with identical parameters. Consider extracting this to a
[TestInitialize]method to improve maintainability.Apply this diff to refactor the common setup:
+ [TestInitialize] + public void Setup() { + var locale = Locale.NA; + Filter.Load(TestUtils.XmlReader, locale.ToString(), "Live"); + } + [TestMethod] public void TestBanWordParser() { - var locale = Locale.NA; - Filter.Load(TestUtils.XmlReader, locale.ToString(), "Live"); var parser = new BanWordParser(TestUtils.XmlReader);Maple2.File.Parser/BanWordParser.cs (1)
19-53: Extract common parsing logic to reduce duplication.
ParseBanWords()andParseUgcBanWords()contain nearly identical logic. The only difference is the file filter predicate. Consider extracting the common parsing logic to a private helper method.Apply this refactor to eliminate duplication:
+ private IEnumerable<(int Id, string Name)> ParseBanWordsInternal(Func<PackFileEntry, bool> fileFilter) { + int i = 0; + foreach (PackFileEntry entry in xmlReader.Files.Where(fileFilter)) { + XmlReader reader = xmlReader.GetXmlReader(entry); + var mapping = nameSerializer.Deserialize(reader) as StringMapping; + + if (mapping == null) { + throw new InvalidOperationException($"Failed to deserialize ban word mapping from entry: {entry.Name}"); + } + + Dictionary<int, string> banWords = mapping.key.ToDictionary(_ => i++, key => key.name); + foreach (var banWord in banWords) { + if (string.IsNullOrEmpty(banWord.Value)) { + continue; + } + yield return (banWord.Key, banWord.Value); + } + } + } + public IEnumerable<(int Id, string Name)> ParseBanWords() { - int i = 0; - foreach (PackFileEntry entry in xmlReader.Files.Where(entry => entry.Name.Contains("banword") && !entry.Name.Contains("ugc"))) { - XmlReader reader = xmlReader.GetXmlReader(entry); - var mapping = nameSerializer.Deserialize(reader) as StringMapping; - - Debug.Assert(mapping != null); - - Dictionary<int, string> banWords = mapping.key.ToDictionary(_ => i++, key => key.name); - foreach (var banWord in banWords) { - if (string.IsNullOrEmpty(banWord.Value)) { - continue; - } - yield return (banWord.Key, banWord.Value); - } - } + return ParseBanWordsInternal(entry => entry.Name.Contains("banword") && !entry.Name.Contains("ugc")); } public IEnumerable<(int Id, string Name)> ParseUgcBanWords() { - int i = 0; - foreach (PackFileEntry entry in xmlReader.Files.Where(entry => entry.Name.Contains("ugcbanword"))) { - XmlReader reader = xmlReader.GetXmlReader(entry); - var mapping = nameSerializer.Deserialize(reader) as StringMapping; - - Debug.Assert(mapping != null); - - Dictionary<int, string> banWords = mapping.key.ToDictionary(_ => i++, key => key.name); - foreach (var banWord in banWords) { - if (string.IsNullOrEmpty(banWord.Value)) { - continue; - } - yield return (banWord.Key, banWord.Value); - } - } + return ParseBanWordsInternal(entry => entry.Name.Contains("ugcbanword")); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Maple2.File.Parser/BanWordParser.cs(1 hunks)Maple2.File.Parser/Maple2.File.Parser.csproj(1 hunks)Maple2.File.Tests/BanWordParserTest.cs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
Maple2.File.Tests/BanWordParserTest.cs (3)
Maple2.File.Parser/Tools/Filter.cs (1)
Filter(9-47)Maple2.File.IO/M2dReader.cs (1)
XmlReader(43-45)Maple2.File.Parser/BanWordParser.cs (2)
BanWordParser(10-54)BanWordParser(14-17)
Maple2.File.Parser/BanWordParser.cs (1)
Maple2.File.IO/M2dReader.cs (1)
XmlReader(43-45)
🔇 Additional comments (3)
Maple2.File.Parser/Maple2.File.Parser.csproj (1)
16-16: LGTM!The version bump from 2.3.3 to 2.3.4 is appropriate for the addition of the new BanWordParser functionality.
Maple2.File.Parser/BanWordParser.cs (2)
14-17: LGTM!The constructor properly initializes the required dependencies for XML deserialization.
27-27: Verify the ID assignment behavior with empty ban words.The current implementation assigns IDs sequentially during dictionary creation (line 27), but only yields non-empty ban words (lines 29-31). This results in non-contiguous IDs when empty entries exist.
For example, if entries 2 and 5 are empty in a set of 10, the yielded IDs will be: 0, 1, 3, 4, 6, 7, 8, 9.
Is this non-contiguous ID assignment intentional? If contiguous IDs are desired, move the increment to the yield statement.
If contiguous IDs are required, apply this diff:
public IEnumerable<(int Id, string Name)> ParseBanWords() { int i = 0; foreach (PackFileEntry entry in xmlReader.Files.Where(entry => entry.Name.Contains("banword") && !entry.Name.Contains("ugc"))) { XmlReader reader = xmlReader.GetXmlReader(entry); var mapping = nameSerializer.Deserialize(reader) as StringMapping; Debug.Assert(mapping != null); - Dictionary<int, string> banWords = mapping.key.ToDictionary(_ => i++, key => key.name); - foreach (var banWord in banWords) { - if (string.IsNullOrEmpty(banWord.Value)) { + foreach (var key in mapping.key) { + if (string.IsNullOrEmpty(key.name)) { continue; } - yield return (banWord.Key, banWord.Value); + yield return (i++, key.name); } } }
Introduces BanWordParser for parsing ban words and UGC ban words from XML files. Adds BanWordParserTest to verify correct parsing and updates project version to 2.3.4.
Summary by CodeRabbit