Queue #10
Queue #10
Conversation
yurii-litvinov
left a comment
yurii-litvinov
left a comment
There was a problem hiding this comment.
С технической точки зрения всё более-менее прилично, но с алгоритмической — качество решения можно охарактеризовать фразой "Заблудились в трёх соснах" :)
| functionResult = queue.Dequeue(); | ||
| Assert.AreEqual(result1[2], functionResult); | ||
| functionResult = queue.Dequeue(); | ||
| Assert.AreEqual(result1[3], functionResult); |
There was a problem hiding this comment.
Честно говоря, не очень понятно, зачем-таки result. Можно было просто проверить на равенство 5, 7 и т.д.
| @@ -0,0 +1,32 @@ | |||
| using System; | |||
|
|
|||
| namespace test2 | |||
There was a problem hiding this comment.
Пространства имён в .NET именуются с заглавной
| public int Priority { get; set; } | ||
|
|
||
| public int Value { get; set; } | ||
|
|
||
| public Node Next { get; set; } |
There was a problem hiding this comment.
Действительно ли им всем нужны public-сеттеры? Ведь если значение попадает в очередь, оно уже не меняется, да и приоритет его тоже.
| private Node tail; | ||
|
|
||
| public Queue() | ||
| => head = null; |
There was a problem hiding this comment.
Он и так null, тем более что tail Вы не инициализируете :)
| node.Next = tail; | ||
| } | ||
|
|
||
| private Node runner; |
There was a problem hiding this comment.
Лучше все поля и все методы объявлять рядом, не перемешивая, тем более что конкретно у этой штуки нет ни одной причины быть полем (она всегда инициализируется в методе перед использованием, следовательно состояние объекта на самом деле не хранит, следовательно могла бы быть локальной переменной).
| /// <summary> | ||
| /// function remove from queue | ||
| /// </summary> | ||
| /// <returns>value's array with max priority</returns> |
There was a problem hiding this comment.
"Массив значения", что? :)
|
|
||
| namespace test2 | ||
| { | ||
| public class QueueException : Exception |
There was a problem hiding this comment.
Тоже нужны комментарии
| } | ||
| if (runner.Priority == maxPriority && runner.Next == null) | ||
| { | ||
| result = runner.Next.Value; |
There was a problem hiding this comment.
runner.Next == null же, иначе мы бы сюда не попали. runner.Next.Value --- это NullReferenceException
| Node previosNode = null; | ||
| while (runner.Next != null) | ||
| { | ||
| if (runner.Next.Priority == maxPriority) |
There was a problem hiding this comment.
Гм. Если самый большой приоритет прячется в голове очереди, что будет? Мы же его пропустим тут?
3 participants
No description provided.