-
Notifications
You must be signed in to change notification settings - Fork 201
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Овечкин Илья #173
base: master
Are you sure you want to change the base?
Овечкин Илья #173
Conversation
public Result<List<TextRectangle>> GetRectangles(Dictionary<string, int> wordFrequencies) | ||
{ | ||
var rectangles = new List<TextRectangle>(); | ||
var bitmap = new Bitmap(imageSettings.Width, imageSettings.Height); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
такие штуки надо не забывать освобождать
return Result.Fail<List<string>>($"The file with the path {filePath} was not found"); | ||
|
||
var words = new List<string>(); | ||
var lines = File.ReadAllLines(filePath); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
вот тут могут еще различные спецеффекты произойти, например не будет хватать прав на чтение, давай оберенм в try catch
if (!interestingWords.IsSuccess) | ||
return Result.Fail<Dictionary<string, int>>(interestingWords.Error); | ||
|
||
foreach (var word in interestingWords.Value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
это все можно сделать в 1 строку
dictionary.GroupBy(x => x).ToDictionary(x => x.Key, x => x.Count())
а вот order кажется уместно делать на месте, сигнатура метода никак не обеспечивает порядка
а то что порядок сохраняется после orderBy
вообще говоря можно сказать достаточно хитрой вопрос на знание внутреннего устройства dictionary, а еще я бы не стал утверждать что так всегда будет, полагаться на такое точно нельзя
|
||
public static void SaveImage(this PictureBox source, string fileName) | ||
{ | ||
source.Image.Save(fileName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
тут наверняка можно ждать ошибок
@@ -0,0 +1,131 @@ | |||
namespace TagsCloudContainer.Infrastucture | |||
{ | |||
public class None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
} | ||
} | ||
|
||
pictureBox.UpdateUi(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
вот тут канеш жесткая связь появилась gui с доменной логикой, я писал уже про то почему это не оч, ладно пусть будет
{ | ||
var builder = new ContainerBuilder(); | ||
|
||
builder.RegisterType<MainForm>().As<Form>().SingleInstance(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
представь что у тебя тысячи файлов, вполне реально в больших проектах
не будешь же так все регистрировать
обычно прибегают к некоторым другим методам
у автофака есть RegisterAllAssemblyTypes()
, не трудно догадаться ка кон работает
а так же используют Module
автофака
руками же регистрируют уже только какие то явные хитрые штуки
@el_razor