abvadabra / itmo-oop-tasks Goto Github PK

Code style style action items:

1. Do not omit brackets when creating one-line loops, methods and so on.
   
   

Optional code style action items:

1. Please remove redundant blank project from solution
   

2. There are unnecessary whitespaces in your files.
   Enable following option in your Rider:
   Settings | Editor | General | Appearance | Show whitespace
   And set Code Inspection: Unnecessary whitespace notifications level to warning.
   

3. You could use expression-bodies properties instead of this:
   
   

There is a TrackGroup class created but was not used, please implement ISearchable interface for TaskGroups too, since it's a task requirement.

All fields of SearchQuery class should be private.
Fields IncludeTracks, IncludeAlbums, IncludeArtists may be converted to auto-properties,
Something like this:

public bool IncludeTracks { get; private set; }

Also, do not ignore your IDE hints:

There is another requirement in the task: Track must not be exists without album, but I can break this logic if i'll call the following statements:

var genre = new Genre("Rock");
var artist = new Artist("My Band", genre);
var track = new Track(artist, null, "Track#01");

Catalog's Artists field should not be public since it's an encapsulation rule violation.
Please review other classes for the same issues.

It would be awesome, if you can make Catalog class static.
That would allow us to register all ISearchable objects in the catalog right after initialization.

This can be simplified
https://github.com/FRedEnergy/itmo-oop-tasks/blob/462e8a0661c991c6d3c211409297273c06c6580b/Lab3/Program.cs#L13-L18
To:

if (!File.Exists(fileName)) {
    Console.WriteLine(fileName + " not found");
    return;
}
var dataFile = IniDataFile.Parse(fileName);

Accoring to MS naming conventions, methods should use UpperCamelCase

Please fix this:
https://github.com/FRedEnergy/itmo-oop-tasks/blob/462e8a0661c991c6d3c211409297273c06c6580b/Lab3/IniDataSection.cs#L19

https://github.com/FRedEnergy/itmo-oop-tasks/blob/462e8a0661c991c6d3c211409297273c06c6580b/Lab3/IniDataSection.cs#L24

p.s. And yet again, IDE should show suggestions like this:

Do not ignore them.

Please take care of culture of numbers with floating-point.

Please use explicit casting from double to string with a culture specification, like so:

var d = 3.123456789;
Console.WriteLine(d.ToString("F2", CultureInfo.InvariantCulture));

I would not advise you to implement string to anything template method in this program.
Try to get rid of Convert.ChangeType and replace it this specific calls for double and ints (i.e. double.TryParse and int.TryParse)
https://stackoverflow.com/questions/30819791/restrict-generic-t-to-specific-types
https://docs.microsoft.com/ru-ru/dotnet/csharp/programming-guide/types/casting-and-type-conversions

https://github.com/FRedEnergy/itmo-oop-tasks/blob/462e8a0661c991c6d3c211409297273c06c6580b/Lab3/IniDataFile.cs#L11-L15
Looks like a leak of verification that section do exists.

If you like syntax sugar, you can use Null coalescing operator:

// ...
get => Sections.GetValueOrDefault(key, null) ?? throw new SectionNotFoundException("...");
// ..

p.s. also, looks like we do not want to have a public setter.

Could you create more exceptions for specific cases?
IniFormatException for formating errors is fine, but we need at least one for cast exceptions and one more for handling invalid structure of fine (i.e. SectionNotFound)