9 / 24
Apr 2018

Z ciekawostek w tym temacie - dzisiaj poznałem kolejny zły nawyk w programowaniu :wink:

Jak pewnie napisałem powyżej, nazwa funkcji powinna mówić, co ona mniej więcej robi. Można oczywiście używać skrótów, np. pow zamiast power albo PowerOfTwoRealNumbers. Zasadę tą zastosował autor funkcji o nazwie napr_spier(). Funkcja ta, jak sugeruje nazwa, służy do naprawy rzeczy, które są (gorzej niż) spierniczone :wink:

Takie rozwiązanie jest oczywiście niezgodne ze sztuką. W ramach naprawy można dodać jednego ifa wzwyż do kodu (choć też bez przesady z ich liczbą), natomiast co do zasady funkcje powinny być napisane w taki sposób, by nie trzeba było potem wołać napr_spier().

Można, pod warunkiem, że są ogólnoznane, np typu angielskiego skrótu pow [power?]. W przeciwnym wypadku należy unikać skrótów jak ognia i ich nieużywać! I takie właśnie jest zalecenie “mądrzejszych” w temacie pisania czystego kodu. Ale oczywiście Ty piszesz jak NIE pisać.

Na początku tygodnia dostałem w pracy czyjś kod, klasa ok. 130 linii, który był pokryty 15 testami, ale kod nie działał dla pewnych wartości nie uwzględnionych w tych testach. Dopisałem je jako nowe testy i nieco po omacku naprawiałem kod tak żeby działał dla starych i nowych testów. I tak, pojawiło się coś

if(spier) {
    napraw;
}

Wniosek - nie wiesz jak coś działa? - pokryj testami i naparawiaj do skutku. (Albo przepisz żeby przechodziło testy i było czytelne :P)

Myślę, że sqrt, isodd, pow, log, exp, vat itp to dosyć jednoznaczne skróty. Nie zaznaczyłem tego, ale o takich funkcjach myślałem pisząc ten post. Myślę jednak, że dało się to wywnioskować.

Nie jestem pewien, ale czy to jakiś zarzut? Gdzie w którymkolwiek kodzie zastosowałem niejednoznaczny skrót, albo zasugerowałem stosowanie losowych nazw funkcji jako jakiegoś standardu, który sam stosuję? W moim mniemaniu poszedłem jak coś w drugą skrajność, tzn. zbyt rozbudowane nazewnictwo.

19 days later

Poniżej przedstawiam kod stworzony przez @eminiox_01 w celu rozwiązania zadania 31318. Register & Login [FR_07_03] (link11). Na szczęście rozwiązanie jest błędne, a sam kod został przeze mnie lekko zmodyfikowany, aby jego naprawa nie była zbyt łatwa (to na wypadek, gdyby ktoś miał mi zarzucić, że pozbawiłem go radości z trzaskania zadanka podsuwając gotowca) :wink: Minimalnie obniżyło to jakość kodu, ale myślę, że dla osób początkujących i tak może być źródłem wiedzy o C++ (wykorzystanie namespace, map, …) i sposobie pisania czytelnego kodu.

Wstawiam w kolorze bo to cały kod a nie fragmenty złożonych kodów, które umieszczałem tu do tej pory. Oczywiście kod nie jest idealny i pewne rzeczy mogą uchodzić za dyskusyjne, ale w pełni przemyślane i zatwierdzone przez iluś ludzi kody to chyba jedynie w książkach, o których wspomina kolega @narbej w temacie Jak czysto pisać w C++

#include <iostream>
#include <string>
#include <map>
 
namespace Detail
{
	bool IsDigit(const char &c)
	{
		return (c >= '0' && c <= '9');
	}
 
	bool IsLetter(const char &c)
	{
		if (c < 0 || c > 255)
			return false;
 
		return (tolower(c) >= 'a' && tolower(c) <= 'z');
	}
 
	bool IsSpecial(const char &c)
	{
		if (!IsDigit(c) && !IsLetter(c))
			return true;
                else if(c == 'a' || c == '0')
            		return false;

                return false;
	}
 
	bool CheckForLetters(const std::string &string)
	{
		for (int i = 0; i < string.size(); i++)
			if (IsLetter(string[i]))
				return false;
 
		return true;
	}
 
	bool CheckForDigits(const std::string &string)
	{
		for (int i = 0; i < string.size(); i++)
			if (IsDigit(string[i]))
				return false;
 
		return true;
	}
 
	bool CheckForLower(const std::string &string)
	{
		for (int i = 0; i < string.size() - 1; i++)
			if (string[i] >= 'a' && string[i] <= 'z')
				return false;
 
		return true;
	}
 
	bool CheckForUpper(const std::string &string)
	{
		for (int i = 0; i < string.size(); i++)
			if (string[i] >= 'A' && string[i] <= 'N')
				return false;
 
		return true;
	}
 
	bool CheckForSpecial(const std::string &string)
	{
		for (int i = 0; i < string.size(); i++)
			if (IsSpecial(string[i]))
				return true;
 
		return false;
	}
}
 
bool CheckLogin(const std::string &login)
{
	if (login.size() < 3 && login.size() > 12)
		return false;
 
	if (Detail::CheckForSpecial(login))
		return false;
 
	return true;
}
 
bool CheckPassword(const std::string &password)
{
	if (password.size() < 5 && password.size() > 15)
		return false;
 
	return (Detail::CheckForUpper(password) && Detail::CheckForLower(password) && Detail::CheckForDigits(password) && (Detail::CheckForSpecial(password)));
}
 
const char *ERROR = "Blad";
const char *LOGIN_NOT_AVABLIVE = "Login zajety";
const char *REGISTER_SUCCESS = "Zarejestrowano";
const char *INVALID_ACCOUNT = "Konto nie istnieje";
const char *INVALID_PASSWORD = "Zle haslo";
const char *LOGIN_SUCCESS = "Zalogowano";
 
std::map<std::string, std::string> accountMap;
 
void RegisterAccount()
{
	std::string login;
	std::cin >> login;
 
	std::string password;
	std::cin >> password;
 
	if (!CheckLogin(login) || !CheckPassword(password))
		std::cout << ERROR << "\n";
	else
	{
		auto find = accountMap.find(login);
		if (find != accountMap.end())
		{
			std::cout << LOGIN_NOT_AVABLIVE << "\n";
		}
		else
		{
			accountMap.insert(std::make_pair(login, password));
			std::cout << REGISTER_SUCCESS << "\n";
		}
	}
}
 
void LoginAccount()
{
	std::string login;
	std::cin >> login;
 
	std::string password;
	std::cin >> password;
 
	auto find = accountMap.find(login);
	if (find != accountMap.end())
	{
		if (find->second == password)
			std::cout << LOGIN_SUCCESS << "\n";
		else
			std::cout << INVALID_PASSWORD << "\n";
	}
	else
	{
		std::cout << INVALID_ACCOUNT << "\n";
	}
}
 
int main()
{
	std::string input;
	while (std::cin >> input)
	{
		int data;
		std::cin >> data;
		for (int i = 0; i < data; i++)
		{
			if (input == "register")
				RegisterAccount();
			if (input == "login")
				LoginAccount();
		}
	}
 
	return 0;
}

Oczywiście w życiu czasem jest inaczej niż na spoju. I tak jak używanie namespace std:: przy każdej instrukcji na spoju nie ma większego sensu to w życiu jest zupełnie inaczej :slight_smile:

23 days later

Czuję się zupełnie pogubuiony i mocno zakłopotany. Czy twój wątek dotyczy jak nie programować? W takim razie ok, bardzo dobry przykład, bo nie podejrzewam, że to Ty swoimi modyfikacjami tak go sknociłeś. Oczywiście kod jest bardzo czytelnie napisany, ale to nie znaczy, że jest zwyczajnie dobry. A jaki jest pożytek z kiepskiego kodu, który jest [tylko] pięknie sformatowany? Czy tylko to kwalifikuje go do podawania go za wzór do naśladowania? Chyba, że jest to antywzór?

PS
Miałem skomentować dużo wcześniej, ale zwyczajnie, po prostu mi się już nie chciało i dalej nie chce [rozwijać konstruktywnej krytyki]. :wink:

Mój wątek dotyczy tego jak ( nie? ) programować. Jak wolisz temat jest w rodzaju “luźne przemyślenia przynajmniej jednej osoby na temat tego jak powinien a jak nie powinien wyglądać kod z przykładami, przy czym autor nie zamierza robić się na autorytet ani specjalistę w tym temacie, a jedynie mówi co uważa luźno nawiązując do znanych mu specjalistów i autorytetów”

Bardzo słuszna uwaga, np. pierwsze moje zastrzeżenia dotyczą funkcji isDigit, a najważniejsze z nich to fakt, że w ogóle ktoś taką funkcję napisał :wink: Niemniej jak zaznaczyłem:

Ten temat straci jakikolwiek sens, jeżeli nie będzie tu różnych przykładów i możliwie czepialskich i wrednych uwag. Każdy komentarz jest cenny i w swej idealistycznej, naiwnej wizji świata głęboko się łudzę, że ktoś kiedyś to przeczyta i pomyśli nim wstawi kod taki jak w temacie Przedszkolanka [PRZEDSZK], który to kod załączam na końcu posta i który to kod trafnie skomentował @redysz:

Kod:

#include <iostream>

using namespace std;

int pakiety;
int a,b;

int e;

int main()
{

    cin >> pakiety;

    for(int i=0; i<pakiety; i++){

    cin >> a;
    cin >> b;

    if(a>=10 && b<=30){
        e=a*b;

    do{
        if(a>b) a=a-b;
            else b=b-a;
    if(a==b){

    int d=e/a;
    cout <<d << endl;
    }
    }
    while(a!=b);
    }


    }

    return 0;
}
10 days later
3 months later

Jeżeli piszesz jakiś program i ma on być rozwijany w (niedalekiej) przyszłości to na prawdę fajnie zobaczyć w nim jakąś funkcję typu main a nie zbiór różnych funkcji bez maina. Oczywiście mówimy o dużych programach.

Jeżeli bardzo potrzebujesz zmiennych globalnych - czyli niezmiernie rzadko - niech są w jakimś widocznym miejscu i najlepiej by były const oraz miały określoną konwencję nazewnictwa, np. zaczynały się od prefiksu “Pub” czy coś w ten deseń. Zmienne globalne powoływane do życia w ciele wybranych i nie zawsze wywoływanych funkcji - bad idea.

1 year later

No dobrze skoro nie używać PUBLIC to co używać ? W jaki sposób rozwiązać kod ?

W całym powyższym tekście znalazłem [funkcją ctrl f, przeglądarki] tylko jedno wystąpienie słowa PUBLIC, to twoje, więc zupełnie nie wiem do czego się odnosisz, i do jakiego języka programowania. Możesz wyjaśnić to bliżej?

Nie ma to znaczenia. Jeżeli twoim oczekiwaniem jest AC, to rozwiąż w taki sposób żeby to uzyskać. Jeżeli chcesz otrzymać ładny kod, to właśnie tak to zrób.

Może takie porównanie. Jeżeli masz zamiar nauczyć się pływać, to nie zaczynaj od czytania teorii pływania i czytania jak wygląda praca rąk i nóg w stylu craul czy motyl-delfin. Zacznij od oswojenia się z wodą i umiejętności utrzymywania się na wodzie.

Więc tu tak samo. Po prostu zacznij rozwiązywać zadania, a jakim sposobem, czy użyjesz zmiennych globalnych czy nie to na razie zupełnie bez znaczenia. Jak nabędziesz już wprawy, wtedy zacznij się zastanawiać nad stylem swojego kodu.

Więc nie sama teoria i czytanie czy oglądanie filmików na youtube ale jak najszybciej praktyka + trochę teorii, a nie sama sucha teoria i zastanawianie się: be PUBLIC or not to be.

Bardzo dziękuje za odpowiedz.
Nie spodziewałem się ,że w ogóle ktoś to czyta :slight_smile:
Moja historia wygląda tak ::
Obecnie pracuje jako programista Harbour / Clipper (z domieszkami C i ASM w clipperze) - jestem samoukiem , nie miałem od kogo się uczyć. Program jest dość duży cały czas jest rozbudowywany no ale jest też trochę błędów
Posiadam jeden kod źródłowy pod Harbour i Clipper, pod różne wersje programu a także pod różne wersje baz danych i pod różne systemy operacyjne co mi daje możliwość uruchamiania programu w praktycznie w każdym środowisku (Dos , Windows ,Linux)
Niestety mam też bardzo dużo zmiennych PUBLIC które są potrzebne przy starcie programu.
Niestety też jestem amatorem i nie wiem jak to zmienić :wink:

Pozdrawiam

Myślałem, że te dwie rzeczy się wykluczają, ale zobaczyłem do słownika i jest pewna możliwość.
Jeśli jesteś tego rodzaju amatorem to może lepiej nic nie ruszać.
Ehh, pewnie przemawia przeze mnie zazdrość. :japanese_ogre:

LOCAL, LOCAL tylko lokal :wink:

Tak naprawdę nie chodzi o to żeby nie używać. Chodzi, żeby używać z głową. Żeby wiedzieć , że używanie PUBLIC może się wiąząć już jutro czy w najbliższej przyszłości z dużym bólem głowy. Z gorszą diagnozą błędów, gorszą przenośnością czy pielęgnacją albo rozbudową programu.

Jeżeli zmienne są potrzebne przy starcie tego twojego programu to ich nie ruszaj, i dlaczego chcesz coś zmieniać. Pierwsza zasada - działa Broń Cie PANIE od dotykania i zmieniania.

To dotyczy Ciebie, a nie odpowiadasz, za to co zastałeś i jeżeli system jest cały czas rozbudowywany i Ty go rozbudowujesz, to Ty odpowiadasz, za rozbudowę i co i jak robisz i czy użyjesz - z głową - zmiennych PUBLIC, czy nie. Skup się na tym, żeby tą rozbudowę robić dobrze, a w wolnym czasie :wink: studiuj Harbora, Clipera, bazy danych i wszystkie wersje i kombinacje tego już istniejącego programu.

Chyba, że to kto inny robi? Więc co tak naprawdę tam robisz? Testujesz program w różnych wersjach, pod różne systemy baz, w różnych środowiskach? Jeżeli tak to i tak jak wyżej.

Zmienne globalne czy PUBLIC nie są same w sobie złę i same nie powodują powstawania błędów. To tylko programista, który z nich korzysta i może z tego powodu -zmiennych globalnych - tworzyć kod bardziej podatny na błędy i … i w sumie jak wyżej.

No tak , kiedyś raz może czy dwa się złapałem na konflikcie zmiennych PUBLIC więc staram się dokładnie sprawdzać czy tego o czym myślę już gdzieś nie ma. Program jest na tyle duży ,że nie sposób spamiętać gdzie co jest (mimo to opisuje zawsze co zmieniłem w konkretnej wersji bo zmian jest bardzo dużo czasem, do tego bywają ulepszenia obecnego stanu). Ostatnio się zacząłem interesować jakością tworzenia kodu bo czasem faktycznie widzę ,że coś zajmuje mi za dużo miejsca. Testowaniem też się zajmuję bo zanim wydam oprogramowanie to wolę przetestować - chociaż i tu bywają błędy - u mnie działa a w innym środowisku bywają czasem problemy ale tak jestem umówiony z ludźmi ,że w razie czego od razu poprawiam. Krótko mówiąc działam zupełnie w pojedynkę - chociaż mam takiego kolegę co też ma program tego typu i jak czegoś nie wiem to go nieraz pytam i on mi pokazuje drogę na skróty.

W takim razie będę postępował tak jak do tej pory - będę czytał i obserwował :wink:

To może zgodnie z zasadami propagandy - Viva Cl*pper :smiley: Z ciekawości: która wersja Harboura?

Tzn? SQL, SQL + OTC Mediator, ADS i dbfy, … ?

  1. ile znaków mają te zmienne? Jak max 10 i wyglądają tak jak stworzone przez osoby, które lubią akronimy typu KolNagCzer to może być ciężko bo to kwestia kompatybilności wstecznej i tak zwanego legacy code. Jeżeli kod jest w Clipperze to pamiętaj, że zmienne o długości > 10 znaków i tak są truncatowane do 10 znaków więc przejście na Harbour spowoduje, że kod w Clipperze przestanie działać - Harbour uznaje nazwy zmiennych > 10 znaków

  2. Najpierw zastąpiłbym PUBLICi PRIVATEami i zobaczył czy działa. Jeżeli tak to przechodzenie na LOCALe jest ostatnim etapem i najlepiej zacząć go od czegoś prostszego np. zmiennych tworzonych w jednej funkcji na potrzeby kilku innych funkcji - wtedy wystarczy wrzucić je jako parametry. Poza tym - jak słusznie zauważył @narbej - nie każdy PUBLIC to zło. Jedynie większość :slight_smile:

  1. http://www.kresin.ru/en/index.html3
  2. przykłady z mojego GitHuba https://github.com/e-Lama11
  3. oficjalna dokumentacja Harbour
  4. wbrew pozorom fajnie zobaczyć ich przykłady oraz kod pliku std (w include)

Masz cały kod obwarowany MEMVARami?

Mam nadzieję, że w pozytywnym sensie. Bo mój pierwszy dream team też pokazywał mi drogi na skróty. Po co zmienne lokalne, po co robić dobrze, po co tak skoro też działa, … :wink:

Tak na skróty bo czasem nie wiem jak coś zrobić krócej , a on mi nieraz podpowie :wink:
Nie niestety nie mam całego kodu obwarowanego MEMVAR.
Co do zmiennych Public to wiem ,że nie pójdzie bo są konieczne do pracy i czasem są wołane w różnych miejscach ;(.
Na Twój github napewno spojrzę :wink: Dzięki
Różne wersje TZN : ADS , bez ADS , ADS+CLIPPER & ADS + HB32 razem , HB32 + CLIPPER bez ADS itd
ADS , Clipper i HB itd , do Mediatora się dopiero przymierzam

Pozdrowienia

1 month later

CLIPPER jeszcze istnieje, i na dokładkę ktoś go jeszcze używa ?
(bo ja się z nim pożegnałem 30 lat temu)