Skip to content

Шагов Владислав#211

Open
ShagovVladislav wants to merge 3 commits intokontur-courses:masterfrom
ShagovVladislav:master
Open

Шагов Владислав#211
ShagovVladislav wants to merge 3 commits intokontur-courses:masterfrom
ShagovVladislav:master

Conversation

@ShagovVladislav
Copy link
Copy Markdown

=> IsSuccess ? func(Value) : Result<TResult>.Fail(Error);

public Result<TResult> Map<TResult>(Func<T, TResult> func)
=> IsSuccess ? Result<TResult>.Ok(func(Value)) : Result<TResult>.Fail(Error);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

В случае ошибки ты можешь вернуть this вместо создания нового объекта Result с той же ошибкой


private Result<Size> CalculateRequiredImageSize(IReadOnlyList<CloudElement> elements, int padding)
{
if (padding < 0 || padding > 1000)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Можно написать padding is < 0 or > 1000

private Result<Size> CalculateRequiredImageSize(IReadOnlyList<CloudElement> elements, int padding)
{
if (padding < 0 || padding > 1000)
return Result<Size>.Fail("Invalid padding value");
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Здесь бы указать валидный диапазон для padding

}
}

public Result SaveToFile(
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Этот метод используется только в тестах. И уже есть IImageSaver

{
public Result Save(Bitmap bitmap, string filePath)
{
if (bitmap == null)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

В проекте включен Nullable enable, то есть если вопросика после Bitmap нет, то сюда и не придёт null. Ну точнее придёт только, если смелый разраб вызовет метод с bitmap!, но тогда null будет на его совести

Короче разрабы дотнета сделали нам nullable enable чтобы не приходилось проверять параметры на null


OpenFile(settings.OutputFile);
}
private static void OpenDirectory(string path)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Тут нужен перенос между методами

private static void OpenFile(string path)
{

Process.Start(new ProcessStartInfo
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Тут наоборот не нужен перенос

}


private static string[] GetDefaultEnglishStopWords()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

тут 2 переноса

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Это бы вообще отсюда убрать в StopWordsFilter

Можно создать там статическое поле вместо метода

public Result<string> Normalize(string word)
{
if (string.IsNullOrWhiteSpace(word) || word.Length < WordsSettings.MinWordLength)
return Result<string>.Ok(string.Empty);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Тут Fail напрашивается

return this;
}

public Result Generate()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Тут бы накинуть [SupportedOSPlatform("windows")]

var settings = ui.Run();


if (!settings.IsSuccess) return;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Лучше назвать settingsResult. А то эта переменная ж хранит не настройки, а результат их получения

}

OpenFile(settings.OutputFile);
OpenFile(settings.Value.OutputFile);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

2 раза value достаётся, можно вынести в переменную

var bitmap = new Bitmap(imageSize.Width, imageSize.Height);
Graphics g = null;

try
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

тут можно написать using (var g = Graphics.FromImage(bitmap)) { /*code*/ }

Comment on lines +116 to +123
return Result<Size>.Ok(new Size(
Math.Max(maxX + padding, 400),
Math.Max(maxY + padding, 200)));
}
catch (Exception ex)
{
return Result<Size>.Fail($"Failed to calculate image size: {ex.Message}");
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

здесь непонятно зачем try catch. Не вижу ситуации, в которой может вылететь exception

Comment on lines +171 to +176
return Result<List<CloudElement>>.Ok(shifted);
}
catch (Exception ex)
{
return Result<List<CloudElement>>.Fail($"Failed to shift elements: {ex.Message}");
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Здесь тоже вроде не должен exception падать никогда

private readonly LayoutSettings layoutSettings;
private readonly VisualizationSettings visualizationSettings;

private string inputFile;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

райдер говорит, что inputFile должен быть nullable, тк в конструкторе он не инициализируется, и на момент использования может быть не задан

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