Skip to content

Comments

Matrix multiplication#1

Open
Andrw-404 wants to merge 15 commits intomainfrom
matrixMultiplication
Open

Matrix multiplication#1
Andrw-404 wants to merge 15 commits intomainfrom
matrixMultiplication

Conversation

@Andrw-404
Copy link
Owner

No description provided.

Comment on lines 5 to 6
namespace ParallelMatrixMultiplication
{

Choose a reason for hiding this comment

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

Suggested change
namespace ParallelMatrixMultiplication
{
namespace ParallelMatrixMultiplication;

Используйте file-scoped неймспейсы, чтобы не создавать лишних уровней вложенности

/// <summary>
/// A user interface class for working with matrix multiplication.
/// </summary>
public class MatrixUserInterface

Choose a reason for hiding this comment

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

Если все методы static, лучше и класс помечать этим ключевым словом

Comment on lines 105 to 108
private static double CalculateMathematicalExpectation(List<double> values)
{
return values.Sum() / values.Count;
}

Choose a reason for hiding this comment

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

для однострочных функций лучше использовать синтаксис лямбд
C# позволяет, а вот Java такого сделать не даст :)

Suggested change
private static double CalculateMathematicalExpectation(List<double> values)
{
return values.Sum() / values.Count;
}
private static double CalculateMathematicalExpectation(List<double> values) => values.Sum() / values.Count;

/// <param name="filePath">The path to save the file.</param>
public void SaveToFile(string filePath)
{
using (StreamWriter writer = new StreamWriter(filePath))

Choose a reason for hiding this comment

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

Suggested change
using (StreamWriter writer = new StreamWriter(filePath))
using StreamWriter writer = new StreamWriter(filePath);

для таких случаев подойдет using declaration: тоже, чтобы не увеличивать уровень вложенности.
довольно удобно, а Java такое сделать тоже не даст

public int this[int rows, int columns]
{
get => this.numbers[rows, columns];
set => this.numbers[rows, columns] = value;

Choose a reason for hiding this comment

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

А зачем здесь setter и зачем методы инициализации объекта матрицы, которые создают и заполняют этот объект Matrix, в результате сохраняя инвариант объекта, сейчас в static-методах вместо отдельных конструкторов?

Static-методы могут по многу раз вызывать, в отличие от конструктора

@@ -0,0 +1,3 @@
1 2 3 4

Choose a reason for hiding this comment

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

А эти файлы здесь нужны?
И подгрузите, пожалуйста, результаты экспериментов по запуску умножения матриц

Comment on lines 21 to 25
this.testMatrixA2x2 = new Matrix(2, 2);
this.testMatrixA2x2[0, 0] = 9;
this.testMatrixA2x2[0, 1] = 4;
this.testMatrixA2x2[1, 0] = 5;
this.testMatrixA2x2[1, 1] = 1;

Choose a reason for hiding this comment

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

это всё просто необязательно через setter-ы делать :)
лучше задать в конструкторе, чтобы не нарушать инкапсуляцию объекта

Comment on lines 66 to 73
for (int i = 0; i < result.Rows; ++i)
{
for (int j = 0; j < result.Columns; ++j)
{
Assert.That(result[i, j], Is.EqualTo(this.firstExpectedResult[i, j]));
}
}
}

Choose a reason for hiding this comment

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

комментарий на будущее: можно написать такой Equals в классе Matrix один раз и потом его везде использовать, чтобы не дублировать этот код

[Test]
public void ParallelMultiplication_NotSquareMatrices_ShouldReturnExpectedResult()
{
var result = MatrixMultiplier.ParallelMultiplication(this.testMatrixA2x3, this.testMatrixB3x2, 4);

Choose a reason for hiding this comment

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

А для оптимального количества потоков есть удобное свойство Environment.ProcessorCount, если, конечно, вам для экспериментов не надо число потоков менять

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