Ocena kodu moich programów

Ocena kodu moich programów
Wyder
WY
Wyder
WY
  • Rejestracja:około 6 lat
  • Ostatnio:około 6 lat
  • Postów:1
0

Witam

Staram się znaleźć jakąś prace bądź staż, choć w większości ofertach pracy wymagane są studia bądź lata doświadczenia komercyjnego, nie mam ani tego, ani tego, gdyż jestem samoukiem po technikum, a chciałbym zjeść na programowaniu zęby.

Prosiłbym was o zerknięcie kod moich przykładowych programów oraz ocenę moich umiejętności na jego podstawie (także o konstruktywną krytykę), dodatkowo o jakieś porady co robić dalej, na co mogę z takimi umiejętnościami liczyć.

No i oczywiście mój GitHub

Z góry dziękuje za odpowiedzi i zmarnowany na mnie czas.

Udostępnij
Kopiuj link do postu Kopiuj link do postu jako Markdown
Komentuj
fasadin
fasadin
fasadin
fasadin
  • Rejestracja:ponad 13 lat
  • Ostatnio:prawie 3 lata
  • Postów:4882
5
  1. piszesz w C z klasami zamiast w C++. Wszystko ma byc obiektowe, nie rob funkcji w main tylko odpowiednia klase ktora sie tym zajmie
  2. uzywasz nagich wskaznikow (poczytaj o RAII)
  3. formatowanie kodu
  4. magic numbers
  5. nazewnictwo void TheWay(RenderWindow * window) TheWay moze tez oznaczac jakiegos rapera ;)
  6. obliczenia w warunkach
Kopiuj
void MindBlow(RenderWindow * window)
{
    Int32 popX = 0;Int32 popY = 0; for(Int32 i = -x/2;i<x/2;i++){if(i != -x/2){ float col = 128*cos((i+clock()%18000)*0.3)+128;
    Vertex line[ 2 ] ={ Vertex( Vector2f( popX+250, -(popY-250) ) ,Color(0,0,0,256*cos((i+clock()%18000)*0.3)-128)),
    Vertex( Vector2f( i+250, -(sin((i+clock()%18000)*0.3)*(i-250-clock()/50)/5-250) ) ,Color(0,0,0,256*cos((i+clock()%18000)*0.3)-128)) };
    window->draw(line,2,Lines);} popX = i; popY = static_cast<int32_t>(-sin((i+clock()%18000)*0.3)*(i-250-clock()/50)/5);}
    for(Int32 i = -x/2;i<x/2;i++){if(i != -x/2) { float col = 128*cos((i+clock()%18000)*0.3)+128;
    Vertex line[ 2 ] = { Vertex( Vector2f( popX+250, -(popY-250) ) ,Color(col,i,col)), Vertex( Vector2f( i+250, -(-sin((i+clock()%18000)*0.3)*(i-250)*8-250) ) ,Color(200,0,0))  };
    window->draw(line,2,Lines); } popX = i;  popY = static_cast<int32_t>(-sin((i+clock()%18000)*0.3)*(i-250)*8);}
}

zgadzam sie z nazewnictwa tej funkcji. Ten kod to po prostu mindblow. Nikt oprocz ciebie go nie zrozumie, za miesiac nawet sam nie bedziesz rozumial

mam wrazenie ze lamiesz DRY

to odnosnie Random-Graphics

odnosnie Game

  1. czemu rozdzielasz include? Jezeli bedziesz miec 1000 plikow to bedziesz miec 1000 plikow w include i include folder bedzie kontenerem na wszystkie headery?
  2. polsko angielskie nazewnictwo. Pisz wszystko po angielsku
  3. formatowanie
  4. srand(time(NULL)); to jest praktycznie depricated. https://stackoverflow.com/questions/19665818/generate-random-numbers-using-c11-random-library
  5. RAII
  6. 1 klasa jeden plik
  7. poczytaj o SOLID
  8. #pragma once zamiast #ifdef
  9. brak testow

dalej mi sie juz nie chce. Popraw to co wyzej to wtedy sprawdze bardziej szczegolowo

edytowany 1x, ostatnio: fasadin
AM
6) 1 klasa jeden plik, rozumiem że to jest rada dla początkujących na zachowanie czytelności, bo jako ogólna rada to uważam że jest dość słaba 8) #pragma once zamiast #ifdef, tu się zupełnie nie zgadzam #pragama once > #ifdef (chyba że piszesz coś co ma szanse być budowane na kompilatorze o nie wspiera once)
fasadin
@amd: przeciez napisalem jasno ze #pragma once jest lepsze a Ty mi piszesz #pragama once > #ifdef ;o odnosnie jedna klasa jeden plik sa wyjatki i powinno sie to trzymac wyjatkami ;)
AM
Ehh zrozumiałem to opacznie. Bo te podpunktu brzmią jak. 'X to zrobiłeś źle', np brak testów. Więc pisząc pragma once zamiast ifdef zabrzmiało. Że to jest źle.
Udostępnij
Kopiuj link do postu Kopiuj link do postu jako Markdown
Komentuj
MarekR22
MarekR22
MarekR22
MarekR22
Moderator C/C++
  • Rejestracja:około 17 lat
  • Ostatnio:2 minuty
0

Ja jeszcze dodam:

  • zależności w repo (SFML) i to w postaci binarnej.
Jeśli chcesz pomocy, NIE pisz na priva, ale zadaj dobre pytanie na forum.
Udostępnij
Kopiuj link do postu Kopiuj link do postu jako Markdown
Komentuj
TobiPL
TobiPL
TobiPL
TobiPL
  • Rejestracja:prawie 11 lat
  • Ostatnio:prawie 3 lata
  • Postów:66
0

Nigdy jeszcze nie widziałem aż takich długich linii kodu...

u siebie w N++ mam taką opcje że jest pasek który ustawiony jest na max 90 znakach
i staram się go nie przekraczać tymczasem twój kod jest sporo za tym paskiem...

masz:

Kopiuj
void MindBlow(RenderWindow * window)
{
    Int32 popX = 0;
	Int32 popY = 0; 
	
	
	for(Int32 i=-x/2;i<x/2;i++)
	{
		if(i!=-x/2)
		{ 
			float col=128*cos((i+clock()%18000)*0.3)+128;
			Vertex line[2]={
				Vertex(Vector2f(popX+250,-(popY-250)),Color(0,0,0,256*cos((i+clock()%18000)*0.3)-128)),
				Vertex(Vector2f(i+250,-(sin((i+clock()%18000)*0.3)*(i-250-clock()/50)/5-250)),Color(0,0,0,256*cos((i+clock()%18000)*0.3)-128))
				};
			
			window->draw(line,2,Lines);
		}
	popX=i; 
	popY=static_cast<int32_t>(-sin((i+clock()%18000)*0.3)*(i-250-clock()/50)/5);
	}
    
	
	
	for(Int32 i=-x/2;i<x/2;i++)
	{
		if(i!=-x/2) 
		{ 
			float col = 128*cos((i+clock()%18000)*0.3)+128;
			Vertex line[2]={
				Vertex(Vector2f(popX+250,-(popY-250)),Color(col,i,col)),
				Vertex(Vector2f(i+250,-(-sin((i+clock()%18000)*0.3)*(i-250)*8-250)),Color(200,0,0))
				};
			
			window->draw(line,2,Lines); 
		} 
	popX=i;  
	popY=static_cast<int32_t>(-sin((i+clock()%18000)*0.3)*(i-250)*8);
	}
}

Nie rozumiem po co taka nazwa i po co tak napisałeś tą funkcję...
ale kiedy zobaczyłem to:

Kopiuj
quad[i*2].position = Vector2f( cos(i*PI/180)*B(D,i/360)*R+250, -(sin(i*PI/180)*B(D,i/360)*R-250));
quad[i*2].color = Color(Z(sin(clo))*c[0] , Z(sin(clo+120*PI/180))*c[0] , Z(sin(clo+240*PI/180))*c[0]);

quad[i*2+1].position = Vector2f( cos(i*PI/180)*B(D,i/360)*R2+250, -(sin(i*PI/180)*B(D,i/360)*R2-250));
quad[i*2+1].color = Color(Z(sin(clo))*c[1] , Z(sin(clo+120*PI/180))*c[1] , Z(sin(clo+240*PI/180))*c[1]);

quad2[i*2].position = Vector2f( cos(i*PI/180)*R2*B(D,i/360)+250, -(sin(i*PI/180)*R2*B(D,i/360)-250));
quad2[i*2].color = Color(Z(sin(clo+rad1))*c[2] , Z(sin(clo+120*PI/180+rad1))*c[2] , Z(sin(clo+240*PI/180+rad1))*c[2]);

quad2[i*2+1].position = Vector2f( cos(i*PI/180)*R3*B(D,i/360)+250, -(sin(i*PI/180)*R3*B(D,i/360)-250));
quad2[i*2+1].color = Color(Z(sin(clo+rad1))*c[3] , Z(sin(clo+120*PI/180+rad1))*c[3] , Z(sin(clo+240*PI/180+rad1))*c[3]);

quad3[i*2].position = Vector2f( cos(i*PI/180)*R3*B(D,i/360)+250, -(sin(i*PI/180)*R3*B(D,i/360)-250));
quad3[i*2].color = Color(Z(sin(clo+rad2))*c[4] , Z(sin(clo+120*PI/180+rad2))*c[4] , Z(sin(clo+240*PI/180+rad2))*c[4]);

quad3[i*2+1].position = Vector2f( cos(i*PI/180)*R4*B(D,i/360)+250, -(sin(i*PI/180)*R4*B(D,i/360)-250));
quad3[i*2+1].color = Color(Z(sin(clo+rad2))*c[5] , Z(sin(clo+120*PI/180+rad2))*c[5] , Z(sin(clo+240*PI/180+rad2))*c[5]);

to pomyślałem sobie... a ok ten MindBlow nie był wcale taki straszny...
przecież to jest jak zbieranina losowych znaków... tego sie nie da czytać xD
Największy moim zdaniem u cb problem to, to jak wygląda ten kod xD
bo człowieku nawet mój nie wygląda tak "Magicznie" :P delikatnie rzecz ujmując...

też odnośnie tego np.

Kopiuj
uint32_t PointConnection::GetBegin()
{
    return BeginPoint;
}

to są gettery i settery które nie mają żadnej kontroli błędów?
całkowicie nie rozumiem napisania funkcji która po prostu zwraca jeden element bez podejmowania żadnych akcji...

ale ok, ja robię zupełnie inne rzeczy w zupełnie innym świecie to przyjmijmy że się nie znam
i ta funkcja ma jakikolwiek sens...

kiedyś widziałem klasę która wyglądała mniej więcej tak:

Kopiuj
class Class{
	private:
		int a,b,d;
	public:
	void SetA(int A){
		a = A;
	}
	void GetA(){
		return a;
	}
}

i tak dla każdej zmiennej z osobna było... a 90% tego było niewykorzystywane...
no ale ok... "Przecież kiedyś może się przydać" xD tsa

czemu też nie ma żadnych komentarzy czy instrukcji?
rozumiem że jak piszesz dla siebie to tam ****** ***** < nie cenzurujcie mnie sam to zrobie >
ale jak pokazujesz komuś kod to przydałoby się dodać skromne instrukcje :P

np.

Kopiuj
bool JustDraw(int y,int x){
	#include "src/JustDraw.cpp"
	/*
	:: This function will just draw image to def. screen
	:: --- Requirments ---
	:: int -> y coord to start drawing
	:: int -> x coord to start drawing
	*/

dzieląc wszystko na pliki nie będziesz musiał brać zapasów miesięcznych w drogę na koniec pliku aby go np. zmodyfikować
i nie musiałbyś brać zapasów na drogę powrotną :P lel

AM
1) 90 linii to limit sprzed 15 lat, gdy full hd jeszcze nie było standardem. (Tak wiem że Google nadal używa tego rozmiaru) 2) 'to są gettery i settery które nie mają żadnej kontroli błędów?całkowicie nie rozumiem napisania funkcji która po prostu zwraca jeden element bez podejmowania żadnych akcji...' nie wiesz co piszesz 3) Twój przykład z JustDraw to moim zdaniem przykład słabego komentarza który nic nie wnosi. (Ale ogólnie rozumiem że to miał być tylko przykład ideii która jest dobra) 4) include w definicji funkcji. To jest po prostu złe.
MarekR22
banalnych getterów i setterów nie powinieneś się czepiać. One mają swoje uzasadnienie, związane z utrzymaniem kodu.
AM
no tak właśnie o to mi chodziło, trivial gety sety mają jak najbardziej sens.
MarekR22
@amd mój komentarz był do TobiPL. A limit kolumn 80 (taki był stary standard) lub coś w okolicy 100, nadal ma sens, bo jak robisz "side by side diff" podczas code review, albo "3-way merge": podczas rozwiązywania konfliktów, to długie linie są strasznie irytujące. FullHd czy QHD nie ma znaczenia, bo mam czytać kod wygodnie, bez lupy lub kręcenia głową.
AM
@MarekR22: doklanie masz kod czytac wygodnie. Jedyny zysk z 90 lini jest przy 3way mergu. Tylko teraz procentowo ile czasu spedzasz przy 3-way mergu, a ile przy normalnym czytaniu kodu. Bo u mnie sytuacje gdzie te 90 lini ma sens to promile. Do tego to jest c++, wiec bardzo łatwo o przekroczenie tych 90 lini. Dlatego nie, absolutnie nie jestem zwolennikiem 90 lini.
MarekR22
Na code review/inspection spędza się dużo czasu i "side by side view" jest czytelniejszy niż standardowy diff! nie spotkałem jeszcze kodu, który byłby ładnie i czytelnie napisany, który nie mieści się w 100 kolumnach. Natomiast kod, który często przekracza ten limit, zwykle ma wszystkie inne mankamenty nieczytelności i jest po prostu crap codem (niestety 90% kodu tak ma). Polecam Robert C Martin "Clean Code" - zamiast dyskusję ze mną, albo popracować w projekcie, gdzie docenia się czysty kod i ludzie dbają o takie rzeczy, które "profesjonalni inaczej" ignorują.
AM
Czytalem Clean Code. Co do review to nie rozumiem tego komentarza. Przy review nie masz 3-way diffa. Tylko 2-way. Nie, do 2-way nie potrzebujesz 90 linii. Spokojnie możesz użyć 120-160. Co do czytelnosci to jest to bardzo subiektywna rzecz. Dla mnie jeśli w mam template ktory jako parametry przyjmuje 2 templaty ktore też moga jakiś template przyjąć to to na 90 linijkach wyglada jak crap. Oczywiscie zawsze możesz napisać, że taki kod jest profesjonalny inaczej.
MarekR22
podczas review z lewej mam drzewko plików zmienionych, w środku plik wyjściowy, a z prawej plik końcowy. Jak coś jest dłuższe niż 100 kolumn to bez horyzontalnego scroll się nie objedzie. Argument Full HD czy QHD jest bezsensu, bo chce mieć font o rozsądnym rozmiarze, żebym lupy nie potrzebował, a monitory większe niż 25'' nie poprawiają wygody pracy. A nawet w IDE często mam podobny układ: drzewo projektu, kod produkcyjny, testy do tego kodu (Xcode potrafi fajnie trzymać te okna zsynchronizowane, jeśli się przestrzega nazewnictwa).
AM
Zawsze możesz korzystać z wordwrapa gdy bardzo Ci zależy żeby ci się kod mieścił w 90 linijkach.
MarekR22
bitbucket nie ma takiej opcji (crucible miało), github nie pamiętam.
AM
github ma na pewno, bitbucket który mam w pracy też ma taką opcje, ale biorąc pod uwagę jakim parchem są apki atlassiana to nie zdziwię się jak defaultowy bez pluginu bitbucket by tego nie miał
Udostępnij
Kopiuj link do postu Kopiuj link do postu jako Markdown
Komentuj
_zbyszek
ZB
_zbyszek
ZB
  • Rejestracja:około 7 lat
  • Ostatnio:ponad 4 lata
  • Postów:23
1

Koledzy powyżej już dość dobrze zwrócili uwagę co jest nie tak.
Aby kod był schludny i łatwiejszy w utrzymaniu oraz czytaniu polecam Ci abyś zapoznał się z książką Clean Code autorstwa R.C. Martina.
Poczytaj też o zasadach SOLID (np. na http://javadeveloper.pl/solid )

edytowany 1x, ostatnio: _zbyszek
Udostępnij
Kopiuj link do postu Kopiuj link do postu jako Markdown
Komentuj
Treść
Podgląd
Kliknij, aby dodać treść...
Kliknij, aby dodać załącznik lub wklej ze schowka.
Instrukcja obsługi Markdown
Pomoc 1.18.8

Typografia

Edytor obsługuje składnie Markdown, w której pojedynczy akcent *kursywa* oraz _kursywa_ to pochylenie. Z kolei podwójny akcent **pogrubienie** oraz __pogrubienie__ to pogrubienie. Dodanie znaczników ~~strike~~ to przekreślenie.

Możesz dodać formatowanie komendami , , oraz .

Ponieważ dekoracja podkreślenia jest przeznaczona na linki, markdown nie zawiera specjalnej składni dla podkreślenia. Dlatego by dodać podkreślenie, użyj <u>underline</u>.

Komendy formatujące reagują na skróty klawiszowe: Ctrl+B, Ctrl+I, Ctrl+U oraz Ctrl+S.

Linki

By dodać link w edytorze użyj komendy lub użyj składni [title](link). URL umieszczony w linku lub nawet URL umieszczony bezpośrednio w tekście będzie aktywny i klikalny.

Jeżeli chcesz, możesz samodzielnie dodać link: <a href="link">title</a>.

Wewnętrzne odnośniki

Możesz umieścić odnośnik do wewnętrznej podstrony, używając następującej składni: [[Delphi/Kompendium]] lub [[Delphi/Kompendium|kliknij, aby przejść do kompendium]]. Odnośniki mogą prowadzić do Forum 4programmers.net lub np. do Kompendium.

Wspomnienia użytkowników

By wspomnieć użytkownika forum, wpisz w formularzu znak @. Zobaczysz okienko samouzupełniające nazwy użytkowników. Samouzupełnienie dobierze odpowiedni format wspomnienia, zależnie od tego czy w nazwie użytkownika znajduje się spacja.

Znaczniki HTML

Dozwolone jest używanie niektórych znaczników HTML: <a>, <b>, <i>, <kbd>, <del>, <strong>, <dfn>, <pre>, <blockquote>, <hr/>, <sub>, <sup> oraz <img/>.

Skróty klawiszowe

Dodaj kombinację klawiszy komendą notacji klawiszy lub skrótem klawiszowym Alt+K.

Reprezentuj kombinacje klawiszowe używając taga <kbd>. Oddziel od siebie klawisze znakiem plus, np <kbd>Alt+Tab</kbd>.

Indeks górny oraz dolny

Przykład: wpisując H<sub>2</sub>O i m<sup>2</sup> otrzymasz: H2O i m2.

Składnia Tex

By precyzyjnie wyrazić działanie matematyczne, użyj składni Tex.

<tex>arcctg(x) = argtan(\frac{1}{x}) = arcsin(\frac{1}{\sqrt{1+x^2}})</tex>

Kod źródłowy

Krótkie fragmenty kodu

Wszelkie jednolinijkowe instrukcje języka programowania powinny być zawarte pomiędzy obróconymi apostrofami: `kod instrukcji` lub ``console.log(`string`);``.

Kod wielolinijkowy

Dodaj fragment kodu komendą . Fragmenty kodu zajmujące całą lub więcej linijek powinny być umieszczone w wielolinijkowym fragmencie kodu. Znaczniki ``` lub ~~~ umożliwiają kolorowanie różnych języków programowania. Możemy nadać nazwę języka programowania używając auto-uzupełnienia, kod został pokolorowany używając konkretnych ustawień kolorowania składni:

```javascript
document.write('Hello World');
```

Możesz zaznaczyć również już wklejony kod w edytorze, i użyć komendy  by zamienić go w kod. Użyj kombinacji Ctrl+`, by dodać fragment kodu bez oznaczników języka.

Tabelki

Dodaj przykładową tabelkę używając komendy . Przykładowa tabelka składa się z dwóch kolumn, nagłówka i jednego wiersza.

Wygeneruj tabelkę na podstawie szablonu. Oddziel komórki separatorem ; lub |, a następnie zaznacz szablonu.

nazwisko;dziedzina;odkrycie
Pitagoras;mathematics;Pythagorean Theorem
Albert Einstein;physics;General Relativity
Marie Curie, Pierre Curie;chemistry;Radium, Polonium

Użyj komendy by zamienić zaznaczony szablon na tabelkę Markdown.

Lista uporządkowana i nieuporządkowana

Możliwe jest tworzenie listy numerowanych oraz wypunktowanych. Wystarczy, że pierwszym znakiem linii będzie * lub - dla listy nieuporządkowanej oraz 1. dla listy uporządkowanej.

Użyj komendy by dodać listę uporządkowaną.

1. Lista numerowana
2. Lista numerowana

Użyj komendy by dodać listę nieuporządkowaną.

* Lista wypunktowana
* Lista wypunktowana
** Lista wypunktowana (drugi poziom)

Składnia Markdown

Edytor obsługuje składnię Markdown, która składa się ze znaków specjalnych. Dostępne komendy, jak formatowanie , dodanie tabelki lub fragmentu kodu są w pewnym sensie świadome otaczającej jej składni, i postarają się unikać uszkodzenia jej.

Dla przykładu, używając tylko dostępnych komend, nie możemy dodać formatowania pogrubienia do kodu wielolinijkowego, albo dodać listy do tabelki - mogłoby to doprowadzić do uszkodzenia składni.

W pewnych odosobnionych przypadkach brak nowej linii przed elementami markdown również mógłby uszkodzić składnie, dlatego edytor dodaje brakujące nowe linie. Dla przykładu, dodanie formatowania pochylenia zaraz po tabelce, mogłoby zostać błędne zinterpretowane, więc edytor doda oddzielającą nową linię pomiędzy tabelką, a pochyleniem.

Skróty klawiszowe

Skróty formatujące, kiedy w edytorze znajduje się pojedynczy kursor, wstawiają sformatowany tekst przykładowy. Jeśli w edytorze znajduje się zaznaczenie (słowo, linijka, paragraf), wtedy zaznaczenie zostaje sformatowane.

  • Ctrl+B - dodaj pogrubienie lub pogrub zaznaczenie
  • Ctrl+I - dodaj pochylenie lub pochyl zaznaczenie
  • Ctrl+U - dodaj podkreślenie lub podkreśl zaznaczenie
  • Ctrl+S - dodaj przekreślenie lub przekreśl zaznaczenie

Notacja Klawiszy

  • Alt+K - dodaj notację klawiszy

Fragment kodu bez oznacznika

  • Alt+C - dodaj pusty fragment kodu

Skróty operujące na kodzie i linijkach:

  • Alt+L - zaznaczenie całej linii
  • Alt+, Alt+ - przeniesienie linijki w której znajduje się kursor w górę/dół.
  • Tab/⌘+] - dodaj wcięcie (wcięcie w prawo)
  • Shit+Tab/⌘+[ - usunięcie wcięcia (wycięcie w lewo)

Dodawanie postów:

  • Ctrl+Enter - dodaj post
  • ⌘+Enter - dodaj post (MacOS)