Conversation
…id class DeckService BusinessLogic #4
includingByMeAndMyself
changed the title
includingByMeAndMyself
force-pushed
the
Dev
branch
from
221a14d to
f2e728d
Compare
|
|
||
| <ItemGroup> | ||
| <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.9.4" /> | ||
| <PackageReference Include="xunit" Version="2.4.1" /> |
There was a problem hiding this comment.
Можно было сразу создать проект с NUnit, это часть стандартных шаблонов :)
А так в целом xunit мне нравится больше. Пожалуй Microsoft он тоже нравится больше, потому что на их проектах использоваться xunit
| private Stack<Card> _cards; | ||
|
|
||
| public int Id { get; set; } | ||
| public Stack<Card> Cards { get { return _cards; } } |
There was a problem hiding this comment.
public Stack Cards => _cards; // можно вот так это написать, выглядит чуть лучше :)
| public Stack<Card> Cards { get { return _cards; } } | ||
|
|
||
| public Suit Name { get; set; } | ||
| public int Count { get; private set; } |
There was a problem hiding this comment.
Если тебе и правда нужен Count, то логичнее его брать из _cards.Count
|
|
||
| public Card(Rank rank, Suit suit, int sequenceNumber) | ||
| { | ||
| Rank = rank; |
There was a problem hiding this comment.
При создании типа я бы еще добавил проверку Enum.IsDefined(rand); Чтобы убедиться, что туда не передали фигню. Просто Enum это такой противный тип данных, в который можно записать (скастить, смаппить, сериализовать) любое число
| { | ||
| c.SwaggerDoc("v1", new OpenApiInfo { Title = "Tyche.API", Version = "v1" }); | ||
| }); | ||
| var filePath = Path.Combine(System.AppContext.BaseDirectory, "Tyche.API.xml"); |
There was a problem hiding this comment.
Советую взять название файла с документацией так
var xmlFilename = $"{Assembly.GetExecutingAssembly().GetName().Name}.xml";
вместо
"Tyche.API.xml"
Просто чтобы уменьшить возможность как то опечататься
There was a problem hiding this comment.
Ну либо сделать константу AppName = "Tyche.API"
так даже лучше :)
Можно будет ее использовать везде, где нужно вставить название апи
|
|
||
| namespace Tyche.BusinessLogic.Infrasturcure | ||
| { | ||
| internal class CardShuffler |
There was a problem hiding this comment.
похоже, что это доменный класс, лучше его унести в домен
и сделать модификатор public.
А еще написать для него юнит тесты :)
| } | ||
| catch (Exception) | ||
| { | ||
| //Log errors or do anything you think it's needed |
|
|
||
| public async Task<bool> AddAsync(Deck deck, string name) | ||
| { | ||
| if (deck == null) return false; |
There was a problem hiding this comment.
Эта проверка лишняя, ниже у тебя string.IsNullOrWhitespace уже проверяет на null
| [StringLength(50, MinimumLength = 3)] | ||
| public string Name { get; set; } | ||
|
|
||
| public ICollection<CardEntity> Deck { get; set; } |
There was a problem hiding this comment.
лучше назвать свойство Cards вместо Deck, а то вызывает путаницу
| using System.Collections.Generic; | ||
| using Tyche.DataAccess.MsSql.Entities; | ||
| using Tyche.Domain.Models; | ||
|
|
2 participants
No description provided.