Hej, proszę o roast mojego GH pod kątem ew. zatrudnienia i jego analizy przez ew. pracodawców.
Czyli co byście zmienili, dodali, odjęli, czego za dużo, za mało, za źle. Konstruktywna krytyka wskazana.
Zaczynam szukać pracy jako C# / ASP.NET / WPF Dev, bez komercyjnego doświadczenia.
Tutaj nie ma kodu źródłowego https://github.com/przemyslawbak/Dluzyna_Szkola
Po co te komentarze do oczywistych rzeczy, dodatkowo po polsku?
https://github.com/przemyslawbak/Mail_24/blob/master/Models/AdresModel.cs
Dodatkowo śmiesznie wyglądają nazwy commitow, np:
Create ZalacznikViewModel.cs
Pisz po angielsku, w ostateczności całość po polsku
Co do reszty, nie piszę w C#, więc może ktoś inny się wypowie też
Path.Combine(), polecam do łączenia ścieżek, bo potem zmieniasz Windows/Linux i coś się może rozsypać (https://github.com/przemyslawbak/Stolarstwo_Ciesielstwo/blob/master/Controllers/HomeController.cs#L21) :-) if (!string.IsNullOrWhiteSpace(adresSerwera) && !string.IsNullOrWhiteSpace(port)
&& !string.IsNullOrWhiteSpace(imieNazwisko) && !string.IsNullOrWhiteSpace(adresEmail)
&& !string.IsNullOrWhiteSpace(haslo) && (adresEmail).Contains('@') && (adresEmail).Contains('.') && regex.IsMatch(port))
Raczej da się to uprościć :P
Formatowanie
Mieszasz polski z angielskim
Często się powtarzasz.
Komentowanie co robi np pętla foreach jest bezcelowe.
Trochę mało jednak na tym gicie.
Dzięki za poświęconą chwilę nad moim kodem oraz feedback. Na taką wiadomość po cichu liczyłem! Czasami coś mogłem przeoczyć, bądź zwyczajnie o czymś nie wiedzieć.
Ktos napisał(a):
- Jeśli już robisz sobie Repository (https://github.com/przemyslawbak/Portfolio_Website/blob/master/Controllers/MyProjectsController.cs#L31), to dlaczego logikę wybierania/sortowania danych i tak masz w kontrolerze? Zrobiłbym raczej bardziej rozbudowane repozytoria, np. tutaj dodając jakąś metodę w stylu GetProjectById().
Słuszna uwaga. W kontrolerach używających Dropdown List, metody zamieniłem na:
var techQuery = repositoryTech.TechnologiesByNames;
if (techQuery == null)
{
Technology newTech = new Technology
{
Name = "change my name",
PictureLink = "#"
};
repositoryTech.SaveTechnology(newTech);
}
A w repozytorium wygląda to tak:
public IEnumerable<Technology> TechnologiesByNames
{
get
{
var technologiesByName = from t in _context.Technologies
orderby t.Name
select t;
return (technologiesByName);
}
}
- Ciekawa funkcja, rzucanie wyjątku na żądanie (https://github.com/przemyslawbak/Portfolio_Website/blob/master/Controllers/MyProjectsController.cs#L47) ;-)
To bezmyślnie wkleiłem z podręcznika. Poprawione.
Path.Combine(), polecam do łączenia ścieżek, bo potem zmieniasz Windows/Linux i coś się może rozsypać (https://github.com/przemyslawbak/Stolarstwo_Ciesielstwo/blob/master/Controllers/HomeController.cs#L21) :-)
Done. Przyznaję, że o tym nie wiedziałem.
- Tytuły commitów masz zawsze niezbyt wiele mówiące - "Create Something.cs". Ale dlaczego? Co to za zmiana? Co to zmienia w projekcie? Czy jeden commit to pojedyncza funkcja twojego systemu? Jeżeli dobrze rozumiem, to nie korzystasz z gita, ale wrzucasz/edytujesz pliki bezpośrednio na GH... a to nie świadczy o tym, że umiesz w ogóle gita używać - a szkoda, bo to bardzo fajne narzędzie. W pewnym sensie będzie mógł ci zastąpić twoje pliki WORK LOG
I tu, jak się okazuje, do końca (albo wcale) jednak nie wiem jak działają Git i GH. Dzisiaj posiedzę nad tym.
- Chyba masz trochę powtarzającego się kodu pomiędzy kontrolerami (https://github.com/przemyslawbak/Portfolio_Website/blob/master/Controllers/LiteratureController.cs#L31), może warto z tego zrobić jedną metodę?
Prawda. Zastanawiam się gdzie ją wrzucić. W Infrastructure?
- Nie rozumiem takiego komentarza kompletnie (https://github.com/przemyslawbak/Portfolio_Website/blob/master/Controllers/LiteratureController.cs#L134)
To akurat był mój roboczy komentarz, odznaczałem sobie metody które chciałem wrzucić w Repo, bazując na książce A. Freemana. Jak się później okazało później, niektóre nie powinny się tam w ogóle znaleźć. Ale to już inna historia.
Z początku wahałem się czy umieścić w GH kontroler logowania, ale z drugiej strony złe doświadczenie to dobre doświadczenie. Szczególnie, że może ktoś zwróci moją uwagę na ew. błędy i może to zaszkodzić jedynie mnie.
- W ogóle masz gdzieś odpaloną tę swoją stronę-portfolio?
Tak. http://przemyslaw-bak.pl/
- Chyba w tym projekcie Portfolio_Website nie ma wszystkich plików tego projektu - na przykład packages.json?
Dorzuciłem.
Dzięki raz jeszcze!
kzkzg napisał(a):
Formatowanie
co konkretnie masz na myśli?
Mieszasz polski z angielskim
W którym projekcie? Czy chodzi może o commity w GH?
Często się powtarzasz.
W jakim przykładzie?
Komentowanie co robi np pętla foreach jest bezcelowe.
O który projekt chodzi?
Trochę mało jednak na tym gicie.
To trochę ciężko mi przeskoczyć, ale pracuję nad tym. Nie chciałem wrzucać na GH "hello worldów", tutoriali, czy testowych kodów. Doszedłem do wniosku, że to by było już "try too hard".
WeiXiao napisał(a):
if (!string.IsNullOrWhiteSpace(adresSerwera) && !string.IsNullOrWhiteSpace(port) && !string.IsNullOrWhiteSpace(imieNazwisko) && !string.IsNullOrWhiteSpace(adresEmail) && !string.IsNullOrWhiteSpace(haslo) && (adresEmail).Contains('@') && (adresEmail).Contains('.') && regex.IsMatch(port))Raczej da się to uprościć :P
Słuszna uwaga, dzięki. Zrobiłbyś to tak?
if ((new[] { adresSerwera, port, imieNazwisko, adresEmail, haslo }).All(c => !string.IsNullOrWhiteSpace(c))
&& (new[] { "@", "." }).Contains(adresEmail) && regex.IsMatch(port))
Czy masz może lepszy pomysł?
Z poprawką zaproponowaną przez @Ktos to nieźle popłynąłeś - https://github.com/przemyslawbak/Stolarstwo_Ciesielstwo/commit/899ab431d044ccd95782f5469cc002ce761d4a2e
To co zrobiłeś nie zmieniło absolutnie nic.
http://przemyslaw-bak.pl/MyProjects/Details?projectID=5
Main features:
The application was originally built on ASP.NET MVC 5 and then converted to ASP.NET Core 2 MVC.
Architectural pattern of MVC was inspired by Adam Freeman's book along with its advantages and disadvantages.
Sory, miałem sobie dać spokój z flame, ale no ur... nie dałem rady :---|
Widać, po wzorcach, które stosujesz i jak je stosujesz, że jesteś początkującym, widać też, że się starasz. Tylko słabe książki czytasz :-|
Pomijając już fakt, że MVC to nie jest wzorzec architektoniczny.
Ja to chyba nawet 1/3 takich ikonek nie mógłbym wstawić na swojej stronie. Muszę się jeszcze dużo uczyć
"fakt, że MVC to nie jest wzorzec architektoniczny" - jako że zaprzeczasz popularnemu w internecie (chociażby wiki) poglądowi, proszę o rozwinięcie
MVC został opracowany przez Trygve Reenskauga w latach 70. Pierwotnie miał być wzorcem Architektonicznym. Można go technicznie zaimplementować w ten sposób, lecz we współczesnej architekturze nie spotkasz go w tej formie. Nawet w książce Fowlera jest on wymieniony jako wzorzec projektowy - przezentacji, oczywiście z uwzględnieniem "tej nieścisłości".
Czy jest w ogóle sens rozróżniania typów wzorców i później kłócenia się o ich definicje? Jakie to ma znaczenie czy jakiś wzorzec jest architektonicznym, projektowym czy dowolnym innym skoro dużo ważniejsze jest to, żeby widzieć kiedy go użyć, kiedy użyć zmodyfikowanego i co ważne kiedy w ogóle nie warto.
@danek: nie chodzi o to, żeby się kłócić o definicje, ale o to, żeby rozumieć co się robi. Masa ludzi "pisze w MVC" i twierdzi, że dzięki temu wzorcowi ich aplikacje są trójwarstwowe, co jest nieprawdą. Gdyby wiedzieli, że MVC to nie jest wzorzec architekoniczny, to by może nie opowiadali takich bzdur.
Czy jest w ogóle sens rozróżniania typów wzorców i później kłócenia się o ich definicje?
Jeden będzie widział sens, drugi nie. Nie widzę sensu w dochodzeniu czy jest sens.
To pytanie o MVC zadałem spodziewając się podobnej odpowiedzi.
Kto powiedział, że wzorzec architektoniczny powinien (musi?) dotyczyć budowy całej aplikacji?
somekind napisał(a):
Masa ludzi "pisze w MVC" i twierdzi, że dzięki temu wzorcowi ich aplikacje są trójwarstwowe
Myślę, że podstawowy błąd jest tutaj
@somekind: w sensie, że MVC to jest wzorzec projektowy? Nawet nie wiedziałem, że są wzorce architektoniczne :p
Twoja stronka strasznie muli, efekty hover/focus działają z jakimś lagiem, obrazek
http://przemyslaw-bak.pl/src/img/projects/stolarz.png?v=g4M85OyLjxscGOd2Vu_3Ms770X-ot5AI20ZpfMPlBN0
ma 1MB
jquery.js ma aż 116kb
trzeba dopieścić temat
Kto powiedział, że wzorzec architektoniczny powinien (musi?) dotyczyć budowy całej aplikacji?
Ja zawsze używam co najmniej 5 wzorców architektonicznych w jednej aplikacji.
Czy jest w ogóle sens rozróżniania typów wzorców i później kłócenia się o ich definicje? Jakie to ma znaczenie czy jakiś wzorzec jest architektonicznym, projektowym czy dowolnym innym skoro dużo ważniejsze jest to, żeby widzieć kiedy go użyć, kiedy użyć zmodyfikowanego i co ważne kiedy w ogóle nie warto.
Masz rację nie musimy rozróżniać wzorców, definicji też nie musimy znać. A po co się kłócić. Przecież kłócą się tylko głupki.
Jak niektórzy już pisali, nie mieszaj polskiego z angielskim pisz wszystko po angielsku (np. nazwy klas są po polsku, kod po angielsku).
Imho. każda klasa powinna być w osobnym pliku.
"foreach (string nazwaWłasności in nazwyWłasności) //dla każdej nazwy własności z nazwyWłasności"
Komentarz niepotrzebny i znowu mieszanie polskiego i angielskiego - załóż, że ten kod może czytać każdy.
Według mnie nie dostaniesz roboty z takim kodem. Poczytaj o dobrych praktykach (OOD, SOLID). Może dostaniesz się na staż. Opanuj algorytmy, struktury danych i przygotuj się na pytania z podstaw języka i SQL.
