Skip to content

First kr first try#14

Open
Palezehvat wants to merge 3 commits intomasterfrom
FirstKRFirstTry
Open

First kr first try#14
Palezehvat wants to merge 3 commits intomasterfrom
FirstKRFirstTry

Conversation

@Palezehvat
Copy link
Owner

Первая контрольная, первая попытка

@@ -0,0 +1,9 @@
namespace PriorityQueue;

public class Program

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Это не надо сразу по двум причинам:

  • Можно было сделать очередь с приоритетами библиотекой (<OutputType>Library</OutputType> в .csproj), и тогда точка входа была бы не нужна вовсе.
  • Если бы точка входа была нужна, её лучше было реализовать с помощью top-level statements, без всяких class Program и Main

/// </summary>
public class Queue
{
private QueueElement? Head;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Поля именуются со строчной

/// <param name="element">Adding element</param>
public void Enqueue(int element, int priority)
{

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Лишняя пустая строчка, обычно после { пустые строки не ставятся

Comment on lines +64 to +67
public bool IsEmpty()
{
return Head == null;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Заказывали свойство Empty, получили метод IsEmpty(). В реальной жизни у кого-то не собрался бы проект, и кто-то получил бы неустойку за ошибку в протоколе.

return;
}
var item = new QueueElement(element, priority);
if (Head.Priority < priority)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nullability-анализ недоволен, сразу минус балл :)

Priority = priority;
}


Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Лишняя пустая строчка


public class Tests
{
Queue queue;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Queue queue;
private Queue queue;

Comment on lines +59 to +63
private static IEnumerable<TestCaseData> QueueForTest
=> new TestCaseData[]
{
new TestCaseData(new Queue()),
};

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Это не нужно, есть же поле queue и SetUp, который его инициализирует. Сейчас получается, что это поле вообще не используется.

public void QueueShouldReturnValueWhenAddedSomething(Queue queue)
{
queue.Enqueue(12, 1);
Assert.True(queue.Dequeue() == 12);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assert.AreEqual или Assert.That

{
new TestCaseData(new Queue()),
};
} No newline at end of file

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Нигде не проверяется, что очередь и вправду очередь, а не просто структура данных, которая запоминает первое значение с максимальным приоритетом. Надо было несколько значений попробовать взять.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments