Parallel matrix multiplication #1
Conversation
| { | ||
| private static List<double> parallelTimesMeans = []; | ||
| private static List<double> sequentialTimesMeans = []; | ||
| private static List<int> sizes = []; |
There was a problem hiding this comment.
По мне так если у класса есть состояние, он не должен быть static. Потому что мутабельные static-поля — это что-то вроде глобальных переменных, которые запутывают поток данных в программе, плюс их сборщик мусора не сможет собрать никогда.
| double[] sequentialTimes = new double[runs]; | ||
| double[] parallelTimes = new double[runs]; |
There was a problem hiding this comment.
Не пишите один и тот же тип дважды
|
|
||
| private static int[][] GenerateMatrix(int size) | ||
| { | ||
| var random = new Random(); |
There was a problem hiding this comment.
Как раз объект типа Random — хороший кандидат на статическое поле, потому что его в принципе более одного на всё приложение не нужно.
| this.result[i] = new int[this.columns]; | ||
| } | ||
|
|
||
| Thread[] threads = new Thread[this.rows]; |
There was a problem hiding this comment.
Нет смысла создавать больше потоков, чем Environment.ProcessorCount, они будут только мешать друг другу.
| private int[][] result = []; | ||
| private int[][] matrix1 = []; | ||
| private int[][] matrix2 = []; | ||
| private int columns = 0; | ||
| private int rows = 0; |
There was a problem hiding this comment.
Эти поля только чтобы в MultiplyRowByColumn параметры не передавать? Мне кажется, оно того не стоит, да и MultiplyRowByColumn можно было не делать, а просто превратить её в лямбду внутри Multiply, тогда эти штуки оказались бы в её замыкании.
| public class Program | ||
| { | ||
| /// <summary> | ||
| /// The main method. | ||
| /// </summary> | ||
| /// <param name="args">Arguments for the program.</param> | ||
| public static void Main(string[] args) |
There was a problem hiding this comment.
Используйте top-level statements всегда, Main в современном C# писать, кажется, вообще больше никогда не надо
| return result; | ||
| } | ||
|
|
||
| private static void EnterMatrix(int[][] matrix) |
There was a problem hiding this comment.
Может, "WriteMatrix" или что-то такое было бы более подходящим тут?
|
|
||
| private static void EnterMatrix(int[][] matrix) | ||
| { | ||
| StreamWriter sw = new ("result.txt"); |
There was a problem hiding this comment.
| StreamWriter sw = new ("result.txt"); | |
| using StreamWriter sw = new ("result.txt"); |
Оно же IDisposable
| [TestCaseSource(nameof(ParallelMultiplicationTestData))] | ||
| public void MatrixMultiplication_ReturnsCorrect(IMatrixMultiplier matrixMultiplier) | ||
| { | ||
| int[][] matrix1 = [[1, 2], [3, 4]]; |
There was a problem hiding this comment.
Надо заглушить предупреждения, раз StyleCop всё ещё не выкатил фикс к этому багу (судя по DotNetAnalyzers/StyleCopAnalyzers#3687, будет в 1.2, который всё ещё в бете)
| yield return new TestCaseData(new SequentialMultiplier()); | ||
| yield return new TestCaseData(new ParallelMultiplier()); | ||
| } | ||
| } |
There was a problem hiding this comment.
Тестов очень мало, нет тестов на исключения, нет на большие матрицы.
| var parallelTimes = new double[runs]; | ||
|
|
||
| var sequentialMultiplier = new SequentialMultiplier(); | ||
| var parallerMultiplier = new ParallelMultiplier(); |
There was a problem hiding this comment.
| var parallerMultiplier = new ParallelMultiplier(); | |
| var parallelMultiplier = new ParallelMultiplier(); |
No description provided.