Skip to content

Conversation

AlexPoziev
Copy link
Owner

No description provided.

@@ -0,0 +1,68 @@
using Trees;

Choose a reason for hiding this comment

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

Неиспользуемый using, уберите, пожалуйста

using Trees;
using LZW;

if (args.Length < 2)

Choose a reason for hiding this comment

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

Если больше 2, то тоже нехорошо вообще-то, добавьте и такую проверку, пожалуйста

using Trees;

/// <summary>
/// class that perfroms encoding by LZW algorithm.

Choose a reason for hiding this comment

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

_performs_все-таки :)

Comment on lines +37 to +38
var newElement = new List<byte>();
newElement.Add((byte)i);

Choose a reason for hiding this comment

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

Suggested change
var newElement = new List<byte>();
newElement.Add((byte)i);
var newElement = new List<byte>
{
(byte)i
};

Comment on lines +42 to +43
var newBytes = new List<byte>();
List<byte> result = new ();

Choose a reason for hiding this comment

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

Так как-то однообразнее будет:

        var newBytes = new List<byte>();
        var result = new List<byte>();

/// <summary>
/// Gets size of Trie, count of elements in Trie, doesn't include empty array.
/// </summary>
public int Size { get { return head.BytesCount; } }

Choose a reason for hiding this comment

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

Suggested change
public int Size { get { return head.BytesCount; } }
public int Size => head.BytesCount;

/// <param name="element">The array to be checked to see if it is contained in.</param>
/// <returns>true -- if Trie contains element, false -- if it doesn't (Empty array contained in the Trie) .</returns>
/// <exception cref="ArgumentNullException">element variable can't be null.</exception>
public bool Contains(List<byte> element)

Choose a reason for hiding this comment

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

Какой-то это уже не бор. Пустой список в боре содержаться не должен по определению. Причем внезапно содержаться он у вас в боре может, а удалить его нельзя. Попробуйте как-то без этого :)

/// <param name="element">element, that need to be added to Trie.</param>
/// <returns>true -- it successfully added the element, false -- the element is already contained (Empty array contained in the Trie).</returns>
/// <exception cref="ArgumentNullException">element can't be null.</exception>
public bool Add(List<byte> element, int value)

Choose a reason for hiding this comment

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

Вы значение, которое передаете вторым аргументов везде называете Value. Тема Value не раскрыта. Без копания в коде совсем не очевидно, что за значение нужно передавать в качестве этого аргумента. Именно поэтому добавьте, пожалуйста, комментарии с пояснениями

/// <param name="prefix">prefix with which the number should start.</param>
/// <returns>Count of elements Trie which start with prefix. Empty array will return count of all words in Trie.</returns>
/// <exception cref="ArgumentNullException">prefix can't be null.</exception>
public int HowManyStartsWithPrefix(List<byte> prefix)

Choose a reason for hiding this comment

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

Ну и так как пустой список в боре не содержится, то и этот метод нужно переделать с учетом этого

namespace LZW;

/// <summary>
/// Class that perfroms decoding by LZW algorithm.

Choose a reason for hiding this comment

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

_performs_все-таки :)

@AlexPoziev AlexPoziev merged commit 7dcc3dc into master May 29, 2023
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.

2 participants