Skip to content

Kr2#55

Open
IgnatSergeev wants to merge 6 commits intomainfrom
kr2
Open

Kr2#55
IgnatSergeev wants to merge 6 commits intomainfrom
kr2

Conversation

@IgnatSergeev
Copy link
Owner

No description provided.

Comment on lines +30 to +32
files.AsParallel()
.ForAll(file => File.Delete(file.path));
Directory.Delete(dirNames[1]);

Choose a reason for hiding this comment

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

Имело смысл это в try/finally обернуть, или вынести в SetUp/TearDown, иначе при исключении в тестируемом коде тест оставит за собой временные файлы.

files.AsParallel()
.ForAll(file => File.WriteAllBytes(file.path, System.Text.Encoding.ASCII.GetBytes(file.data)));

Assert.That(compute(dirNames[0]), Is.EqualTo(compute(dirNames[0])));

Choose a reason for hiding this comment

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

Анализатор ругается, а не должен. Можно было бы переписать этот фрагмент кода, чтобы убрать предупреждение.

}

[Test, TestCaseSource(nameof(ComputeInstances))]
public void FileNameMattersTest(Func<string, byte[]> compute)

Choose a reason for hiding this comment

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

По условию как раз имя файла в формуле для хеша не участвовало, но ладно, это скорее дефект условия

Comment on lines +156 to +158



Choose a reason for hiding this comment

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

Лишние пустые строки

Comment on lines +45 to +47
var fileTasks = Directory.GetFiles(path).Select(ComputeFileAsync);

var dirTasks = Directory.GetDirectories(path).Select(ComputeDirAsync);

Choose a reason for hiding this comment

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

Directory.GetFiles не фиксирует порядок возвращаемых файлов, так что их сначала надо было отсортировать, иначе значение хеша будет зависеть от непредсказуемых внешних факторов (типа удалили и добавили тот же файл).


var overallBytes = (await Task.WhenAll(tasks))
.SelectMany(byteArray => byteArray)
.Concat(ComputeString(new DirectoryInfo(path).Name))

Choose a reason for hiding this comment

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

По условию от имени директории хеш не считается, она добавляется как есть. Впрочем, это особо не важно, просто лишняя работа.

return MD5.HashData(byteArray);
}

private static byte[] ComputeFile(string path)

Choose a reason for hiding this comment

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

Однопоточная версия тоже могла бы (да и должна была, по идее) использовать ComputeFileAsync. Асинхронность и многопоточность — разные вещи.


private static async Task<byte[]> ComputeFileAsync(string path)
{
var byteArray = (await MD5.HashDataAsync(new FileInfo(path).OpenRead())).Concat(ComputeString(new FileInfo(path).Name)).ToArray();

Choose a reason for hiding this comment

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

Чтобы всё было вообще идеально в плане асинхронности, надо было ещё CancellationToken принимать.

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