Cześć, w najbliższym czasie chciałbym dostać się na staż C# .NET web dev.
Stworzyłem swój pierwszy projekt, aby mieć co pokazać, link do projektu (MVC, MSSQL)
Nie testowałem czy na innym kompie zadziała backup bazy.
Na forum czytałem, że to jeden z lepszych sposobów, aby stworzyć większy(jak na początkującego) projekt i poprosić o code review.
Stąd moja prośba, jeśli ktoś bardziej doświadczony mógłby zerknąć, byłbym wdzięczny.
Pozdro.
Ja bym osobiście w takim projekcie nie korzystał z plików bak do odtwarzania bazy, tylko utworzył inicjalizowanie bazy danych za pomocą ModelBuildera (https://docs.microsoft.com/pl-pl/ef/core/modeling/data-seeding#model-seed-data). Wrzucasz takie przykładowe dane tam, robisz migrację i odpalasz metodę Migrate przy odpalaniu aplikacji. Wtedy wystarczy ustawić poprawny connection string i odpalenie aplikacji sprawi, że wykonają się wszystkie migracje razem z załadowaniem danych do bazy.
@lukaszek016:
No to w sumie zależy jak dużo danych tam ma być. Bo swojego czasu robiłem zadanko gdzie musialem na wstępie dodać 400 tyś przykładowych rekordów i wtedy taki model builder nie podoła. I trzeba było pisać SQL'a i wywołać przez EF.
Wracając do tematu to:
Tak na szybko:
Brak await async.
Katalog models a w nim services.
W pętli pobierasz dane z bazy. Może lepiej pobrac wszystkich userów zrobić update i savechanges.
Brak Unit testów.
@KriZz37: możesz robić Update-Database w konsoli lub w kodzie wywołać metodę Migrate podczas uruchamiania aplikacji i jeśli są jakieś migracje do wdrożenia to je wdroży.
C# 
.NET twójDbContext.Database.Migrate();
@szydlak: jasne, są sytuacje, kiedy trzeba kombinować i nie wszystko da się ogarnąć, jednak przy dodawaniu dużej ilości danych to żeby nie zaśmiecać kodu to wrzuciłbym do projektu gdzieś plik ze skryptem SQL, stworzył pustą migrację i dopisał do niej wykonanie tego skryptu SQL.
public void WithdrawMoney(decimal amountToWithdraw, ApplicationUser user)
{
var amount = amountToWithdraw;
var balanceAfter = user.Billing.Balance -= amount;
user.AccountHistory = new List<AccountHistoryEntity>
{
new AccountHistoryEntity
{
ActionType = "wypłata",
Amount = (double)amount,
BalanceAfter = (double)balanceAfter,
Date = DateTime.Now
}
};
dbContext.SaveChanges();
}
Dlaczego przy wyciąganiu pieniędzy historia jest nadpisywana historią z jednym wpisem?
var user = dbContext.Users.Include(x => x.Billing).SingleOrDefault(x => x.Id == id);
if (data.AmountToWithdraw > user.Billing.Balance)
{
ModelState.AddModelError("", "Nie możesz wypłacić więcej pieniędzy, niż masz na koncie");
return View(data);
}
Czy tego checka nie chciałbyś wynieść z kontrolera do np. tego kodu obsługującego wyciąganie kasy?
No bo teraz testując WithdrawMoney mógłby przejść test gdzie wyciągasz kasę z pustego konta i podobne
wow ale cuda na gh :o
szydlak napisał(a):
Brak await async.
Coś czytałem o asynchroniczności, ale czuje, że to nie na mój poziom, zostałem zmuszony do użycia await/async, bo Identity posiada niektóre metody tylko asynchroniczne, nie wiem gdzie popełniłem błędy.
Katalog models a w nim services.
Z tego co zrozumiałem/wyczytałem w kontrolerze mają być przekierowania do metod w serwisach, czyli jak najmniej kodu, jak możesz to proszę rozwiń co jest nie tak.
W pętli pobierasz dane z bazy. Może lepiej pobrac wszystkich userów zrobić update i savechanges.
public async Task<IdentityResult> EditUsersInRole(List<UserRoleViewModel> model, string roleName)
{
var role = await roleManager.FindByNameAsync(roleName);
IdentityResult result = null;
for (int i = 0; i < model.Count; i++)
{
var user = await userManager.FindByIdAsync(model[i].UserId);
if (model[i].IsSelected && !(await userManager.IsInRoleAsync(user, role.Name)))
{
result = await userManager.AddToRoleAsync(user, role.Name);
}
else if (!model[i].IsSelected && await userManager.IsInRoleAsync(user, role.Name))
{
result = await userManager.RemoveFromRoleAsync(user, role.Name);
}
else
{
continue;
}
}
return result;
}
Jeśli chodzi o to, to próbowałem, ale nie umiem inaczej.
Brak Unit testów.
To kolejna rzecz, o której tylko słyszałem, aktualnie odpuszczam, zdaje sobie sprawę, że to ważna sprawa i mnie nie ominie.
WeiXiao napisał(a):
> public void WithdrawMoney(decimal amountToWithdraw, ApplicationUser user)
> {
> var amount = amountToWithdraw;
> var balanceAfter = user.Billing.Balance -= amount;
>
> user.AccountHistory = new List<AccountHistoryEntity>
> {
> new AccountHistoryEntity
> {
> ActionType = "wypłata",
> Amount = (double)amount,
> BalanceAfter = (double)balanceAfter,
> Date = DateTime.Now
> }
> };
>
> dbContext.SaveChanges();
> }
> ```
>
> Dlaczego przy wyciąganiu pieniędzy historia jest nadpisywana historią z jednym wpisem?
>
> ```csharp
> var user = dbContext.Users.Include(x => x.Billing).SingleOrDefault(x => x.Id == id);
> if (data.AmountToWithdraw > user.Billing.Balance)
> {
> ModelState.AddModelError("", "Nie możesz wypłacić więcej pieniędzy, niż masz na koncie");
> return View(data);
> }
> ```
>
> Czy tego checka nie chciałbyś wynieść z kontrolera do np. tego kodu obsługującego wyciąganie kasy?
>
>
> No bo teraz testując `WithdrawMoney` mógłby przejść test gdzie wyciągasz kasę z pustego konta i podobne
Chyba udało mi się poprawić, z tym nadpisywaniem też działało, pewnie dzięki EF, ale wydaje mi się, że teraz jest lepiej.
Dla wynagrodzeń też poprawiłem + dodałem pobieranie użytkowników z czasem większym od 0, zamiast sprawdzać każdego if'em.
```csharp
var users = dbContext.Users.Include(x => x.Billing).Include(x => x.AccountHistory)
.Where(x => x.Billing.MinutesWorked > 0);
Jeszcze był tam błąd logiczny w if'ie data.AmountToWithdraw > user.Billing.Balance poprawiłem na balanceAfter < 0
Tu poprawiony kod, który wspominałeś.
public IActionResult Withdrawal(WithdrawalViewModel data, string id)
{
if (!ModelState.IsValid)
{
return View(data);
}
if (data.AmountToWithdraw == 0)
{
ModelState.AddModelError("", "Wpisz kwotę powyżej 0");
return View(data);
}
var error = userDataService.WithdrawMoney(data, id);
if (error)
{
ModelState.AddModelError("", "Nie możesz wypłacić więcej pieniędzy, niż masz na koncie");
return View(data);
}
return RedirectToAction("Index");
}
public bool WithdrawMoney(WithdrawalViewModel data, string userId)
{
var user = dbContext.Users.Include(x => x.Billing).Include(x => x.AccountHistory)
.SingleOrDefault(x => x.Id == userId);
var amount = data.AmountToWithdraw;
var balanceAfter = user.Billing.Balance -= amount;
if (balanceAfter < 0)
{
return true;
}
user.AccountHistory.Add(new AccountHistoryEntity
{
ActionType = "wypłata",
Amount = (double)amount,
BalanceAfter = (double)balanceAfter,
Date = DateTime.Now
});
dbContext.SaveChanges();
return false;
}
Nie wiem jak dodać ModelState w innym miejscu, niż w kontrolerze, dlatego był taki spaghetti code, zrobiłem zwracanie error jako bool, nie wiem czy to "ma ręce i nogi".
Swoją drogą, dla mnie bez różnicy czy piszę kod w kontrolerze, czy serwisach, przypuszczam, że powinno się w tych serwisach min. do robienia testów i czytelności kodu?
Dzięki za pomoc chłopaki :)
zrobiłem zwracanie error jako bool
Możesz utworzyć swoją klasę obsługującą zwracanie błędów i zwracać w niej więcej informacji niż sam sukces/fail np. komunikat błędu.
na szybko tylko googlnąłem bloga jakiegoś chyba Ruska, więc pewnie jest sensownie opisane, ale chodziło mi o przykład takiej klasy
CTRL+F public class Result
Zarejestruj się i dołącz do największej społeczności programistów w Polsce.
Otrzymaj wsparcie, dziel się wiedzą i rozwijaj swoje umiejętności z najlepszymi.