Rekruter otwiera Twoje repozytorium na GitHubie. Patrzy na jeden plik — kontroler albo serwis. I już wie.
Nie po tym ile kodu napisałeś. Po tym jak go napisałeś.
W tym poście omówię dokładnie 5 błędów które pokazałem w odcinku “Code Review na żywo” na kanale dev-hobby.pl. Do każdego dostajesz: zły kod, wzorcową poprawkę i jedną regułę do zapamiętania.
| 💡 Powiązane wideo: Code Review na żywo — Czy Twój kod w C# wygląda jak kod Juniora? → youtube.com/@mariuszjurczenko |
Co sprawdza rekruter — zanim w ogóle przejdzie do kodu
Rekruter patrzy na Twój kod nie po to, żeby sprawdzić czy działa. Zakłada, że działa. Sprawdza cztery rzeczy:
- Nazewnictwo — czy rozumiesz co piszesz. Widać to po nazwach zmiennych i klas.
- Czytelność — czy myślisz o tym, kto będzie czytał Twój kod po Tobie.
- Zasady SOLID — Single Responsibility, separation of concerns.
- Testowalność — czy Twój kod w ogóle da się przetestować.
Teraz pokażę Ci 5 błędów które łamią każdą z tych zasad — i jak je naprawić.
Błąd #1 — Złe nazewnictwo
Masz prosty kod. Wygląda krótko, wygląda czysto. Ale co on robi? Bez czytania każdej linii — nie wiesz.
❌ Zły kod — Manager, DoStuff, GetThing
// ❌ KOD JUNIORA
public class Manager
{
public List<string> data = new List<string>();
public void DoStuff(string x) // DoStuff — co robi?
{
if (x != null)
data.Add(x);
}
public string GetThing(int i) // GetThing — co zwraca?
{
return data[i];
}
}Manager — manager czego? Emaili? Zamówień? Użytkowników?
Nie wiesz bez czytania środka.
DoStuff to chyba najgorsza nazwa metody jaką rekruter może zobaczyć.
GetThing podobnie — co to za rzecz?
Rekruter widząc takie nazwy myśli: ten developer nie myśli o tym kto będzie czytał jego kod. A to sygnał, że trudno będzie z nim pracować w zespole.
✅ Dobry kod — nazwy które mówią co robią
// ✅ PO POPRAWCE — nazwa mówi co klasa robi
public class UserEmailRepository
{
private readonly List<string> _userEmails = new();
public void AddEmail(string email) // jasne: dodaje email
{
if (!string.IsNullOrWhiteSpace(email))
_userEmails.Add(email);
}
public string GetEmailAt(int index) // jasne: email pod indeksem
=> _userEmails[index];
}📌 Reguła: nazwa powinna mówić CO coś robi, nie JAK to robi.
Bez czytania ciała metody — wiesz co robi AddEmail.
Błąd #2 — God Method / SRP
Drugi błąd jest może jeszcze bardziej niebezpieczny — bo kod wygląda jakby działał, i działa. Ale jest nienaprawialny.
❌ Zły kod — ProcessOrder robi 4 rzeczy naraz
// ❌ KOD JUNIORA — jedna metoda robi WSZYSTKO
public void ProcessOrder(Order order)
{
// 1. Walidacja
if (order == null) throw new Exception("Brak zamówienia");
if (order.Items.Count == 0) throw new Exception("Puste zamówienie");
// 2. Obliczenie kwoty
decimal total = 0;
foreach (var item in order.Items)
total += item.Price * item.Quantity;
if (order.HasDiscount) total *= 0.9m;
// 3. Zapis do bazy ❌ hardcoded connection string
var conn = new SqlConnection("Server=localhost;Database=shop");
conn.Open();
var cmd = new SqlCommand("INSERT INTO Orders...", conn);
cmd.ExecuteNonQuery();
// 4. Wysyłka emaila
var smtp = new SmtpClient("smtp.gmail.com");
smtp.Send("no-reply@shop.pl", order.Email, "Zamówienie",
$"Dziękujemy! Kwota: {total:C}");
}Ta metoda robi cztery rzeczy naraz: waliduje, liczy, zapisuje do bazy i wysyła email. Nie da się jej przetestować. Nie da się zmienić jednej odpowiedzialności bez ryzyka zepsucia pozostałych.
Jak chcę napisać test dla obliczenia kwoty — muszę mockować bazę danych i serwer SMTP. To jest niemożliwe do sensownego testowania.
✅ Dobry kod — SRP w praktyce
// ✅ PO POPRAWCE — każda klasa ma JEDEN obowiązek
public class OrderProcessor
{
private readonly IOrderValidator _validator;
private readonly IPriceCalculator _calculator;
private readonly IOrderRepository _repository;
private readonly IEmailService _emailService;
public OrderProcessor(
IOrderValidator validator,
IPriceCalculator calculator,
IOrderRepository repository,
IEmailService emailService)
{
_validator = validator;
_calculator = calculator;
_repository = repository;
_emailService = emailService;
}
public async Task ProcessAsync(Order order)
{
_validator.Validate(order); // SRP
var total = _calculator.Calculate(order); // SRP
await _repository.SaveAsync(order, total); // SRP
await _emailService.SendConfirmationAsync(order, total); // SRP
}
}
// Każda klasa ma JEDEN powód do zmiany:
// OrderValidator → zmieniają się reguły walidacji
// PriceCalculator → zmienia się logika cen/rabatów
// IOrderRepository → zmienia się sposób zapisu do bazy
// IEmailService → zmienia się format/provider emaila
// OrderProcessor → zmienia się FLOW (kolejność kroków)📌 Reguła: jedna klasa = jeden powód do zmiany.
Jeśli zmieniają się wymagania emaila — zmieniam tylko EmailService.
Reszta kodu nie wie że cokolwiek się zmieniło.
Błąd #3 — Magic strings i hardcoded values
Trzeci błąd jest subtelny — kod działa, wygląda zwięźle, ale jest bombą zegarową.
❌ Zły kod — “admin”, “vip”, hasło w kodzie
// ❌ KOD JUNIORA — magic strings
// literówka? kompilator nie sprawdzi
public decimal GetDiscount(User user)
{
if (user.Role == "admin") return 0.15m;
if (user.Role == "vip") return 0.10m;
if (user.Role == "premium") return 0.05m;
return 0m;
}
// ❌ Hardcoded connection string z hasłem w kodzie źródłowym
var conn = new SqlConnection(
"Server=localhost;Database=shop;User=sa;Password=Admin123");Wyobraź sobie że string “admin” jest w 15 miejscach projektu. Ktoś zmienia rolę na “administrator”. Musi przeszukać cały projekt. O jednym miejscu zapomni — i masz buga.
A connection string z hasłem Admin123 hardcoded w kodzie? To nie tylko zły design — to katastrofa bezpieczeństwa. To trafi do Git repozytorium.
✅ Dobry kod — enum, stałe, konfiguracja
// ✅ Enum zamiast stringa — kompilator sprawdzi czy rola istnieje
public enum UserRole { Admin, Vip, Premium, Standard }
public static class DiscountRates
{
public const decimal Admin = 0.15m; // zmiana w jednym miejscu
public const decimal Vip = 0.10m;
public const decimal Premium = 0.05m;
}
public decimal GetDiscount(User user) => user.Role switch
{
UserRole.Admin => DiscountRates.Admin,
UserRole.Vip => DiscountRates.Vip,
UserRole.Premium => DiscountRates.Premium,
_ => 0m
};
// ✅ Connection string z konfiguracji — NIE z kodu
var connectionString = configuration.GetConnectionString("ShopDb");
// Hasło pochodzi z appsettings.json / secrets.json / env variables
// Nigdy nie trafia do repozytorium Git📌 Reguła: jeśli wartość pojawia się w więcej niż jednym miejscu
— to enum lub stała. Hasła i connection strings
— zawsze w konfiguracji (appsettings.json + user-secrets), nigdy w kodzie.
Błąd #4 — Brak obsługi błędów / null
Czwarty błąd to coś co każdy Junior robi — i co kiedyś doprowadzi do NullReferenceException na produkcji o 3 w nocy.
❌ Zły kod — zwraca null, caller zapomni sprawdzić
// ❌ KOD JUNIORA — zwraca null bez żadnej informacji
public User GetUser(int id)
{
return _db.Users.FirstOrDefault(u => u.Id == id);
// Co jeśli user nie istnieje? Zwraca null.
// Caller musi pamiętać żeby to sprawdzić.
// Zazwyczaj zapomni.
}
// Użycie — bomba zegarowa:
var user = GetUser(999);
Console.WriteLine(user.Name); // NullReferenceException!
SendWelcomeEmail(user.Email); // NullReferenceException!✅ Dobry kod — trzy wzorce obsługi braku wartości
✅ Wariant 1 — rzuć wyjątek z kontekstem (brak = błąd logiki)
// Użyj gdy: brak elementu = błąd logiki biznesowej
public User GetUserOrThrow(int id)
{
return _db.Users.FirstOrDefault(u => u.Id == id)
?? throw new UserNotFoundException($"User id={id} nie istnieje.");
}✅ Wariant 2 — TryGet pattern (brak = normalny przypadek)
// Użyj gdy: brak = normalny przypadek (optional lookup)
public bool TryGetById(int id, out User? user)
{
user = _users.FirstOrDefault(u => u.Id == id);
return user is not null;
}
// Użycie:
if (!TryGetById(999, out var user))
return NotFound("User nie istnieje");✅ Wariant 3 — Result<T> (złożone operacje z różnymi błędami)
// Użyj gdy: operacja może się nie powieść z różnych powodów
public Result<User> TryFind(int id)
{
var user = _users.FirstOrDefault(u => u.Id == id);
return user is not null
? Result<User>.Ok(user)
: Result<User>.Fail($"User id={id} nie istnieje");
}📌 Reguła: nigdy nie zwracaj null bez poinformowania callera
jakie są możliwe stany.
Caller powinien wiedzieć co robić w każdym przypadku.
Wybór wzorca zależy od kontekstu — ważne jest że wybierasz świadomie.
Błąd #5 — Publiczne pola zamiast właściwości
Ostatni błąd — pozornie mały detal, ale dla rekrutera to wyraźny sygnał że ktoś nie zna C#.
❌ Zły kod — publiczne pola bez kontroli
// ❌ KOD JUNIORA — publiczne pola
public class Product
{
public string Name; // każdy może zmienić bez kontroli
public decimal Price; // ktoś może wpisać -999
public int Stock; // ktoś może wpisać -1000
}
var product = new Product();
product.Price = -999; // kompilator nie protestuje
product.Stock = -1000; // niespójny stan — i nikt nie wie✅ Dobry kod — właściwości z walidacją w konstruktorze
// ✅ PO POPRAWCE — właściwości z private set + walidacja
public class Product
{
public string Name { get; private set; }
public decimal Price { get; private set; }
public int Stock { get; private set; }
public Product(string name, decimal price, int stock)
{
if (string.IsNullOrWhiteSpace(name))
throw new ArgumentException("Nazwa nie może być pusta");
if (price <= 0)
throw new ArgumentOutOfRangeException(nameof(price), "Cena > 0");
if (stock < 0)
throw new ArgumentOutOfRangeException(nameof(stock), "Stock >= 0");
Name = name;
Price = price;
Stock = stock;
}
public void ReduceStock(int quantity) // dedykowana metoda dla zmiany stanu
{
if (quantity > Stock)
throw new InvalidOperationException("Niewystarczający stan");
Stock -= quantity;
}
}📌 Reguła: właściwości + private set + walidacja w konstruktorze
= immutable by default.
Obiekt rodzi się poprawny albo w ogóle nie istnieje.
To jest enkapsulacja w praktyce — jeden z fundamentów OOP.
Checklist — sprawdź swój kod przed Pull Requestem
| ✅ Pre-PR Checklist — C# Code Review |
| ☐ Nazwy klas, metod i zmiennych mówią CO robią — nie JAK |
| ☐ Żadna metoda nie robi więcej niż jednej rzeczy (SRP) |
| ☐ Brak magic strings — używam enumów i stałych (DiscountRates.Admin zamiast “admin”) |
| ☐ Brak hardcoded credentials — connection strings i klucze w konfiguracji |
| ☐ Każde null/brak obsługuję świadomie: throw, TryGet lub Result<T> |
| ☐ Właściwości z private set zamiast publicznych pól |
| ☐ Walidacja w konstruktorze — obiekt rodzi się poprawny |
Podsumowanie — 5 błędów w jednym miejscu
| #1 | Złe nazewnictwo Manager, DoStuff, GetThing — rekruter nie wie co robi klasa. Użyj nazw które mówią CO, nie JAK. |
| #2 | God Method (SRP) Jedna metoda robi 4 rzeczy — niemożliwa do przetestowania. Każda klasa = jeden powód do zmiany. |
| #3 | Magic strings “admin” zamiast enuma, hasło w kodzie. Literówka niewidoczna dla kompilatora, ryzyko bezpieczeństwa. |
| #4 | Null bez kontekstu Zwrócone null bez poinformowania callera = NullReferenceException na produkcji. Używaj throw/TryGet/Result. |
| #5 | Publiczne pola Brak enkapsulacji — obiekt może wejść w niespójny stan. Właściwości + walidacja w konstruktorze. |
Każdy z tych błędów sam w sobie nie dyskwalifikuje.
Ale razem mówią rekruterowi: ta osoba nie myśli jeszcze jak developer.
Każdy z tych błędów sam w sobie nie dyskwalifikuje. Ale razem mówią rekruterowi: ta osoba nie myśli jeszcze jak developer.
| 📥 Pobierz bezpłatną Roadmapę Junior .NET Developer 12 kroków od podstaw C# do pierwszej pracy jako .NET Developer. → dev-hobby.pl Masz pytanie do konkretnego błędu? Zostaw komentarz pod filmem — odpowiadam na każdy. |
Zobacz też
- SOLID w C# — 5 zasad na 5 przykładach z prawdziwego projektu
- Clean Architecture w ASP.NET Core
- 50 pytań rekrutacyjnych Junior .NET — z wzorcowymi odpowiedziami
- Dependency Injection w ASP.NET Core
Zobacz także — powiązane artykuły
👉 MCP w .NET (C#) – jak zbudować serwer AI krok po kroku
👉 Tworzenie klas i obiektów w C# — kompletny przewodnik
👉 Pattern Matching w C# – switch expressions i type patterns
CTA
📌 Chcesz więcej praktycznego C# i .NET?
👉 sprawdź moje kursy .NET i C#.
Dołącz do “Od Zera do .NET Developera”
Zacznij swoją przygodę z programowaniem w oparciu o sprawdzone praktyki rynkowe. Wybierz kompletną ścieżkę rozwoju i zbuduj solidne fundamenty.
Dołącz do ścieżki teraz →