Skip to content
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

Ватлин Алексей #30

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

Alex-Vay
Copy link

No description provided.

Comment on lines 5 to 8
public interface IImageColoring
{
public Color GetNextColor();
}

Choose a reason for hiding this comment

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

Такой интерфейс не позволит опираться на какие-то качества слов. Что, например, если я хочу раскрашивать самые популярные красным цветом?

Comment on lines 16 to 31
public string GenerateTagCloud()
{
var text = reader.ReadLines();
foreach (var filter in filters)
text = filter.FilterText(text);

var frequencyDict = text
.GroupBy(word => word)
.OrderByDescending(group => group.Count())
.ToDictionary(group => group.Key, group => group.Count());

var maxFrequency = frequencyDict.Values.Max();
var tagsList = frequencyDict.Select(pair => ToWordSize(pair, maxFrequency)).ToList();

return saver.SaveImage(imageCreator.CreateBitmap(tagsList));
}

Choose a reason for hiding this comment

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

А почему тут путь и не указывать? А то он по дефолтному пути всегда где-то лежит. И возвращать ничего не придется

{
public Color GetNextColor()
{
var random = new Random();

Choose a reason for hiding this comment

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

Создается на каждом обращении. При вызове на 100000 слов создаст 100000 объектов


var fullImageName = $"{imageName}.{imageFormat}";
image.Save(fullImageName);
return Path.Combine(Directory.GetCurrentDirectory(), fullImageName);

Choose a reason for hiding this comment

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

Лучше задавать путь сохранения извне опционально, если его нет - генерить дефолтный

Comment on lines 8 to 18
public class ImageCreator(
Size size,
FontFamily family,
IImageColoring background,
IImageColoring coloring,
ICircularCloudLayouter layouter)
{
public ImageCreator(ImageSettings settings, ICircularCloudLayouter layouter)
: this(settings.Size, settings.Font, settings.BackgroundColor, settings.Coloring, layouter)
{ }

Choose a reason for hiding this comment

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

Конкретно мне кажется, что такой конструктор только засоряет код. Я вижу привычный конструктор, а оказывается там на основании одной абстракции создается объект.

@@ -0,0 +1,7 @@
namespace TagsCloudVisualization.FileReaders.Filters;

public class LowercaseFilter : IFilter

Choose a reason for hiding this comment

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

Это не фильтр. Фильтр не преобразует входные параметры, а именно фильтрует

private int MIN_FONTSIZE = 20;
private int MAX_FONTSIZE = 100;

public string GenerateTagCloud()

Choose a reason for hiding this comment

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

Откуда я должен знать что это за строчка? А это оказывается путь к файлу.

builder.RegisterInstance(SettingsFactory.BuildPolarSpiralSettings(settings)).AsSelf();
builder.Register(context => SettingsFactory.BuildPointLayouterSettings(
context.Resolve<IPointsGenerator>())).AsSelf();
//builder.RegisterInstance(SettingsFactory.BuildSquareSpiralSettings(settings)).AsSelf();

Choose a reason for hiding this comment

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

Лишнее

Comment on lines 55 to 65
builder
.RegisterType<TxtFileReader>().As<IFileReader>()
.OnlyIf(_ => Path.GetExtension(settings.FilePath) == ".txt");

builder
.RegisterType<CsvFileReader>().As<IFileReader>()
.OnlyIf(_ => Path.GetExtension(settings.FilePath) == ".csv");

builder
.RegisterType<WordFileReader>().As<IFileReader>()
.OnlyIf(_ => Path.GetExtension(settings.FilePath) == ".docx");

Choose a reason for hiding this comment

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

Лучше в фабрику


namespace TagsCloudVisualization.CloudLayouter;

public class CloudGenerator(

Choose a reason for hiding this comment

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

Обязательно требование:
"Сделай так, чтобы по одному тексту можно было сгенерировать несколько облаков тегов с помощью разных алгоритмов или одного алгоритма с разными настройками."

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