Feature/phrase search #6
Conversation
| ArgumentNullException.ThrowIfNull(searcher); | ||
| ArgumentNullException.ThrowIfNull(parser); | ||
| _searcher = searcher; | ||
| _parser = parser; |
There was a problem hiding this comment.
_searcher = searcher ?? throw new ArgumentNullException(nameof(searcher));
There was a problem hiding this comment.
به نظرمون روش ما readabilityش بالاتره
باز اگه دلیل خاصی براش دارین لطفا بیشتر توضیح بدین
| var optionalWords = new List<Keyword>(); | ||
| var excludedWords = new List<Keyword>(); | ||
|
|
||
| var regex = new Regex(@"[+-]?\b\w+\b|[+-]?""[^""]+"""); |
There was a problem hiding this comment.
این میتونه const تعریف بشه یا حتی یک سرویس داشته باشیم که regexی که میخوایم رو provide میکنه
بهتره یه summary داشته باشیم که این ریجکس چیکار میکنه
05-tdd/src/App/Utilities/Logging.cs
Outdated
|
|
||
| public static class Logging | ||
| { | ||
| public static ILogger Logger{get; private set;} |
There was a problem hiding this comment.
میتونه جنریک تعریف بشه که برای هر کلاس تایپ اونو بدیم
| HashSet<string> AllDocuments { get; } | ||
| HashSet<string> GetDocumentsByKeyword(Keyword keyword); |
There was a problem hiding this comment.
اگه بخوایم مکان هر کلمه توی سند رو هم بدونیم و بخوایم توسعه بدیم لازم میشه ریترن تایپ رو عوض کنیم
در صورتی که بجای string میتونستیم یه تایپ از کلاس جدیدی که wrapper برای string هست ریترن کنیم (یعنی کلاسی که داخلش متغیر شماره سند باشه و بعدا بتونیم مثلا شماره خط یا.. هم اضافه کنیم)
There was a problem hiding this comment.
صرفا برای اینکه گوشه ذهنمون باشه گفتم نیاز به تغییرات نیست
| private bool Equals(AdvancedInvertedIndex other) | ||
| { | ||
| if (_invertedIndexMap.Count != other._invertedIndexMap.Count) | ||
| return false; | ||
|
|
||
| foreach (var kvp in _invertedIndexMap) | ||
| { | ||
| if (!other._invertedIndexMap.TryGetValue(kvp.Key, out var otherSet)) | ||
| return false; | ||
|
|
||
| if (!kvp.Value.SetEquals(otherSet)) | ||
| return false; | ||
| } | ||
|
|
||
| var areDocumentsEqual = AllDocuments.SetEquals(other.AllDocuments); | ||
| return areDocumentsEqual; | ||
| } | ||
|
|
||
| public override bool Equals(object? obj) | ||
| { | ||
| if (ReferenceEquals(null, obj)) return false; | ||
| if (ReferenceEquals(this, obj)) return true; | ||
| if (obj.GetType() != this.GetType()) return false; | ||
| return Equals((AdvancedInvertedIndex)obj); | ||
| } |
There was a problem hiding this comment.
چرا GetHashCode رو اورراید نکردیم؟ ارتباطش با Equals چیه؟
There was a problem hiding this comment.
نیازه این کلاس IEquatable رو هم ایمپلمنت کنه؟
| Assert.Equal(["doc1"], documents1); | ||
| Assert.Equal(["doc2"], documents2); |
There was a problem hiding this comment.
میشه زد ولی خوانایی همین به نظرمون بهتره
|
|
||
| yield return | ||
| [ | ||
| "-arshad -arshada ali", |
|
|
||
| [Theory] | ||
| [MemberData(nameof(GetTestData))] | ||
| public void ExtractKeywords_ShouldTokenizeText_WhenStringContainsSingleQuotation(string text, List<Keyword> expectedKeywords) |
There was a problem hiding this comment.
فکر کنم single quote + space باید باشه با توجه به تست کیس اول
There was a problem hiding this comment.
بهتر نبود delimitter رو کسی که میخواد tokenize کنه مشخص کنه
No description provided.