Skip to content

Hw2 Lzw#4

Open
MikePuzanov wants to merge 7 commits intomasterfrom
hw2LZW
Open

Hw2 Lzw#4
MikePuzanov wants to merge 7 commits intomasterfrom
hw2LZW

Conversation

@MikePuzanov
Copy link
Copy Markdown
Owner

No description provided.

Comment thread hw2LZW/hw2LZW/LZW.cs Outdated
using System.Collections;
using System.IO;

namespace hw2LZW
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Пространства имён в .NET именуются всегда с заглавной, и пространство имён всего проекта называется по умолчанию так же, как сам проект, поэтому и проекты стоит называть с заглавной

Comment thread hw2LZW/hw2LZW/LZW.cs
}
}
return 1;
}

This comment was marked as resolved.

Comment thread hw2LZW/hw2LZW/Trie.cs Outdated
}

private bool CheckAdd(byte value, Node node)
=> node.IsFind(value) == null ? true : false;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

? true : false можно не писать, == и так возвращает булевое значение

Comment thread hw2LZW/hw2LZW/Trie.cs Outdated
if (runner == root)
{
var check = runner.Sons[value];
if (check.IsUsed == false)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if (check.IsUsed == false)
if (!check.IsUsed)

Comment thread hw2LZW/hw2LZW/LZW.cs
if (codeOfBytes != -1)
{
codes.Enqueue(codeOfBytes);
trie.IsAdd(byter);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

И снова добавляем байт в дерево? Хм, зачем?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Надо придумать и написать этому рациональное объяснение. В реальном проекте нельзя просто взять и проигнорировать комментарий ревьюера.

Comment thread hw2LZW/hw2LZW/Trie.cs
private bool CheckAdd(byte value, Node node)
=> node.IsFind(value) == null ? true : false;

public int LastCode { get; set; }
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

По-хорошему, к свойствам (особенно с неочевидными названиями, как это) тоже надо комментарии

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Не исправлено

Comment thread hw2LZW/hw2LZW/Trie.cs Outdated
/// вернем последний код
/// </summary>
/// <returns>возвращается последний код</returns>
public int GetLastCode()
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

А зачем, есть же public-свойство, которое делает то же самое

Comment on lines +30 to +31
string path1 = "..\\..\\..\\TestTxt.txt";
string path2 = "..\\..\\..\\TestTxtCopy.txt";

This comment was marked as resolved.

Comment thread hw2LZW/hw2LZW/Program.cs
Comment on lines +18 to +19
var compressedFileSize = new FileInfo(args[0]);
var decompressedFileSize = new FileInfo(args[0] + ".zipped");
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Наоборот же :)

hw2LZW.LZW.Compress(path1);
hw2LZW.LZW.Decompress(path1 + ".zipped");
File.Delete(path1 + ".zipped");
Assert.IsTrue(Compare(path1, path2));
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Я проделал это вручную и, если удалить исходный TestImg.jpg, получил:
TestImg

Есть подозрение, что тест проходит только потому, что TestImg.jpg уже существует, а распаковщик не перезаписывает файл до конца (то есть в хвосте остаются байты оригинального изображения).

Copy link
Copy Markdown
Collaborator

@yurii-litvinov yurii-litvinov left a comment

Choose a reason for hiding this comment

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

С одной стороны, стало лучше, с другой стороны, стало хуже :)

Comment thread hw2LZW/hw2LZW/LZW.cs
if (codeOfBytes != -1)
{
codes.Enqueue(codeOfBytes);
trie.IsAdd(byter);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Надо придумать и написать этому рациональное объяснение. В реальном проекте нельзя просто взять и проигнорировать комментарий ревьюера.

Comment thread hw2LZW/hw2LZW/Trie.cs

public bool IsUsed { get; set; }

public int IdByte;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Я пишу "у этих свойств видимость уж слишком широкая, сеттер можно было убрать", а Вы такой "о, почему бы не превратить свойство в public-поле?". Типа "сгорел сарай, гори и хата"? Нет, не прокатит.

Comment thread hw2LZW/hw2LZW/Trie.cs
private bool CheckAdd(byte value, Node node)
=> node.IsFind(value) == null ? true : false;

public int LastCode { get; set; }
Copy link
Copy Markdown
Collaborator

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