Our site uses cookies. Learn more about their purpose and change of settings in a browser If you are using the site, you give you consent to use cookies, according to current browser settings. Got it

Zły kod zabija

  • 15-03-2017

Zły kod zabija

Zły programista John popełnił błąd w kodzie, z powodu którego każdy użytkownik programu musiał spędzić dodatkowe 15 minut by znaleźć obejście tego błęu. Użytkowników programu było 10 milionów. W sumie zmarnowano 150 milionów minut = 2.5 miliona godzin. Jeśli zakładamy, że człowiek śpi 8 godzin na dobę, to na aktywną działalność pozostaje mu 16 godzin. Tak więc John zmarnował 156250 godzin ludzkich ≈ 427.8 lat. Przeciętny mężczyzna żyje 73 lat, a więc John ”zabił” około 5.86 ludzi.

Jak śpisz po nocach, John - seryjny programisto?

Zasady dobrego kodu

To prostota, jasność, kompaktowość, wydajność, brak powtórzeń.

Jeśli tworzysz coś bardziej skomplikowanego, niż «hello world”, to kod zostanie umieszczony nie w jednym, a kilku plikach. Z reguły takich plików jest co najmniej 10. Jeżeli każdy plik jest nazwany niezrozumiałym, krótkim ciągiem znaczków (programiści uwielbiają skróty), to osoba, która będzie chciała spróbować się odnaleźć w kodzie, będzie długo klęła.

Uwaga: «hello world” to nie błąd. Dokładnie tak umieścił nawiasy LibreOffice Writer. Johnny, cześć! Zawsze jest miło Cię widzieć! Żartuję, nigdy.

Jak wybrać dobrą nazwę

Dobra nazwa pliku powinna być jak najkrótsza, a jednocześnie maksymalnie dokładnie opisywać zawartość dokumentu. Inni ludzie będą czytać twój kod, a więc tytuły plików powinny być zrozumiałe dla każdego. 

john_zły_programista to przykład  dobrej nazwy. Dobra nazwa składa się z jednego do trzech słów. Trzy to nawet dużo, a jedno może być niewystarczające, aby opisać zawartość. Spróbuj przeczytać następujące nazwy. Czy będziesz w stanie powiedzieć, jakiego typu kod mogą zawierać te dokumenty?

profiler.h
jitter_measurement.h
developer_menu.h
Animation2D.h
Rectangle.h
Wind_effect.h

Nazwa obiektu w środku pliku jest taka sama jak nazwa samego pliku. Nie róbmy bałaganu. Naprawdę nie ma już potrzeby nadawania nazwy typu SoMFVec2s.h.

Czy w nazwach plików potrzebne są prefiksy?

NIE.

Wizualny szum rozprasza.

Współczesne "edytory tekstowe dla programistów" pomogą uzyskać informacje o jakimkolwiek prototypie/metodzie/klasie/etc po jednym kliknięciu czy najechaniu myszką. Żadne prefiksy w roku 20NN nie są potrzebne.

To samo odnosi się i do przestrzeni nazw — również są prefiksem. Ciągłe korzystanie z prefiksów tylu std:: cv:: oczywiście potrafi chronić przed błędem użycia obiektu z niewłaściwej przestrzeni nazw, ALE...

Czy popełniłeś podobny błąd chociaż raz?

Czy ten błąd miał konsekwencje?

Wybieranie zbędnych symboli zajmuje czas, warto więc wycenić, czy ta procedura się opłaca. Spędziłeś 100 godzin życia na wybieraniu nazw prefiksów? Mogłeś pójść na spacer. 

Czy można dawać zmiennym krótkie nazwy?

TRZEBA.

int i;
char c;
byte b;
string s;
vector v;
pointer p;
argument a1;

Krótkie nazwy są idealne, ponieważ pozwalają zapobiec szumowi. Są czytelne i zrozumiałe. 

Podprzestrzeń

W starszych wersjach Windows w okienku “Panel sterowania Windows” widzimy ikonki “Instalacja aktualizacji Windows”, “Serwisy Windows” oraz “Czcionki Windows”. Dlaczego każdy z tych elementów zawiera słowo "Windows"? Wydaje mi się, że każdy pamięta jakiego systemu używa.

W podprzestrzeni znajdują się 52 ikony, które nie są pogrupowane w jakikolwiek sposób. A pomocy w postaci wyszukiwania po “kilku literkach na klawiaturze” również wtedy nie było. Nie musimy iść tą drogą.

Korzystaj z podprzestrzeni rozumnie: nazwa biblioteki, nazwa katalogu, nazwa pliku, namespace, nazwa klasy, nazwa funkcji. Kiedy ktoś czyta twoją funkcję, poprzednio przeszedł już przez 5 poziomów nazw. Programiści naprawdę potrafią zapamiętać kontekst.

Komentarze

Jeśli kod jest zrozumiały i zawiera przejrzyste nazwy, to mało co będzie w nim trzeba komentować - może nieoczywiste miejsca czy hacki. Lepiej spędzić czas na napisanie krótkiego opisy funkcjonalności modułu. 

Przy okazji

…. Do naszego programu dołóżmy setkę innych, absolutnie nieprzydatnych. Będziemy zmuszać użytkownika do zainstalowania wszystkich, nie dając mu prawa wyboru czy by je chciał czy nie. Swoje główne zadanie natomiast zrobimy byle jak, nie przykładając się — coś z tego powinno wyjść! 

Dzięki temu mamy teraz bardziej skomplikowany i niestabilny system. Cudownie.

O! A może podczas instalacji aktualizacji systemu będziemy zajmować 6 gigabajtów pamięci, tak aby użytkownikowi przestały działać inne odpalone w tym czasie programy. Niech dokupi sobie pamięci. Oczywiście aktualizacja będzie się odbywała wolno nawet na najnowszych komputerach.

Stwórzmy dla naszego systemu operacyjnego masę nowych serwisów, z których nikt nie będzie korzystał. Wszystkie włączymy domyślnie. Oczywiście serwisy te będą bardzo źle zabezpieczone, zwłaszcza na poziomie zdalnego wykonywania kodu - żebyśmy mogli cały czas te zabezpieczenia poprawiać! Czy wspomnieliśmy już o tym, że nasz program instalacji aktualizacji będzie potrzebował gigabajty pamięci i 100% zasobów procesora?

Będziemy popełniać błędy. Niech aktualizacja KB3136000 zainstaluje się kilkakrotnie. John, jesteś geniuszem!

Skomplikowane!

Załóżmy, że mamy proste zadanie - podzielić wiersz danymi separatorami. Proste zadanie powinno zostać rozwiązane w najprostszy sposób:
 

void split(const string& s, std::vector<string>& result, const string& delimiters = " ")
{
    string::size_type a = s.find_first_not_of(delimiters, 0);
    string::size_type b = s.find_first_of(delimiters, a);

    while (string::npos != b || string::npos != a)
    {
        result.push_back(s.substr(a, b - a));

        a = s.find_first_not_of(delimiters, b);
        b = s.find_first_of(delimiters, a);
    }
}

Uprościłem funkcję, usuwając powtórzenie:

void split(const string& s, std::vector<string>& result, const string& delimiters = " ")
{
    string::size_type a, b = 0;

    for (;;)
    {
        a = s.find_first_not_of(delimiters, b);
        b = s.find_first_of(delimiters, a);

        if (string::npos == b && string::npos == a)
            break;

        result.push_back(s.substr(a, b - a));
    }
}

Jeśli wiesz, jak można rozwiązać zadanie w prostszy sposób — zrób to. W internecie możemy znaleźć mnóstwo rozwiązań tego problemu: boost, wyrażenia regularne, statek kosmitów… Co będzie lepsze: gigantyczny boost, który zajmie kilka gigabajtów miejsca na dysku SSD czy mała funkcja? “Ale przecież później to może się przydać do czegoś!”

Kochany przyjacielu, ale ja tylko muszę podzielić linijkę! Keep It Simple, Stupid.

Czyja to wina, że Johnny nie przejmuje się obecnością w C++ najprostszego rozwiązania? John, ilu programistów już zabiłeś? 

O prędkości kodu

Johnny, który tworzy własne "środowisko programistyczne" do profilowania wydajności jest najgorszym typem mordercy! Jego kod nie robi nic pożytecznego od wielu lat. Ludzie tymczasem tracą godziny, próbując wydobyć z tego tworu trochę sensu. 

Konstrukcja tego typu jest znacznie ładniejsza i bardziej wszechstronna od jakiejkolwiek Johnny'ego: 

unsigned long t0 = current_time();

// some code

cout << current_time() - t0 << endl;

Poniżej przedstawiam mój własny profiler, który przechodzi z projektu do projektu od wielu lat w trochę zmieniającej się postaci w zależności od systemów operacyjnych i sposobów wyświetlania informacji na ekranie:


/*
Profiler prof;

for (;;)
{
	Sleep(50); // code, which does not need to measure performance

	prof(NULL);

	Sleep(100); // some code

	prof("code1");

	Sleep(200); // some code

	prof("code2");

	prof.periodic_dump(5);
		// every 5 seconds will print table
}
*/

#include <stdint.h>
#include <stdio.h>

#include <string>
using std::string;

#include <set>
using std::set;

#include <algorithm>
using std::min;
using std::max;

#ifdef WIN32
#include <Windows.h>

class Microseconds
{
public:
	uint64_t operator()()
	{
		LARGE_INTEGER now;
		QueryPerformanceCounter(&now);
		LARGE_INTEGER freq;
		QueryPerformanceFrequency(&freq);

		return now.QuadPart * 1000 / (freq.QuadPart / 1000);
			// overflow occurs much later
	}
};

#else
#include <sys/time.h>
//#include "android_workarround.h"
class Microseconds
{
public:
	uint64_t operator()()
	{
		timeval tv;
		gettimeofday(&tv, NULL);
		return (uint64_t)tv.tv_sec * 1000000 + tv.tv_usec;
	}
};
#endif

class Profiler
{
	Microseconds microseconds;

	class Event
	{
		public:
		const char* name;
		uint64_t time;
		uint64_t count;
		uint64_t min_time;
		uint64_t max_time;

		void reset()
		{
			time = 0;
			count = 0;
			min_time = (uint64_t)-1;
			max_time = 0;
		}
	};

	class Comparator
	{
		public:
		bool operator()(const Event& a, const Event& b) const
		{	//return strcmp(a.name, b.name) < 0;
			return (void*)a.name < (void*)b.name;
		}
	};

	set<Event, Comparator> events;

	uint64_t t0;
	uint64_t last_dump;

	Event c;
	set<Event>::iterator i;

public:
	Profiler()
	{
		last_dump = t0 = microseconds();
	}

	void operator()(const char* what)
	{
		if (what == NULL)
		{
			t0 = microseconds();
			return;
		}

		uint64_t t = microseconds() - t0;

		c.name = what;
		i = events.find(c);

		if (i == events.end())
		{
			c.reset();
			i = events.insert(c).first;
		}

		Event& e = const_cast<Event&>(*i);

		e.time += t;
		e.min_time = min(e.min_time, t);
		e.max_time = max(e.max_time, t);
		++e.count;

		t0 = microseconds();
	}

	void dump()
	{
		const float MS = 0.001f;

		float f_summ = 0;
		for (i = events.begin(); i != events.end(); ++i)
			f_summ += (float)i->time;

		if (f_summ == 0) return;

		f_summ *= MS;
		f_summ *= .01f; // %

		printf("           name count   total(%%)        min   avg   max\n");

		for (i = events.begin(); i != events.end(); ++i)
		{
			Event& e = const_cast<Event&>(*i);

			if (e.count == 0) e.min_time = 0;

			float f_time = e.time * MS;
			float f_min = e.min_time * MS;
			float f_max = e.max_time * MS;
			float f_average = e.count == 0 ? 0 : f_time / (float)e.count;

			printf("%15s %5llu %7.1f(%5.1f%%) %5.1f %5.1f %5.1f\n",
				e.name, (long long unsigned int)e.count,
				f_time, f_time / f_summ, f_min, f_average, f_max);

			e.reset();
		}
	}

	void periodic_dump(unsigned int period)
	{
		if (microseconds() < last_dump + period * 1000000) return;
		dump();
		last_dump = microseconds();
	}
};

W konsoli wygląda to następująco (czas mierzony w milisekundach) 

          name count   total(%)        min   avg   max 
      detector     0     0.0(  0.0%)   0.0   0.0   0.0 
     predictor   161   287.8( 46.1%)   1.0   1.8   2.3 
       refiner   161   246.9( 39.5%)   0.8   1.5   1.8 
     shape fit   161    90.0( 14.4%)   0.3   0.6   0.8

Zwróćcie uwagę na to, że w komentarzu na górze podałem działający przykład. 

Jeśli chcę wypisać tabelę na ekran, to funkcję dump modyfikuję tak aby przekierował wynik na vector out.

Czym jest instalacja programu

Pewnymi pre-skryptowanymi akcjami, które prowadzą do stworzenia “dobrego środowiska” dla programu. W odróżnieniu od “złego środowiska” - w którym program się nie włączy. To oznacza jedno. Programista przyznanie się, że jego program jest na tyle zły, że jest potrzebna preinicjalizacja na kilka sposobów. Popsuło się? Zrestartuj. Nadal nie działa? Zainstaluj ponownie. 

Opublikowałeś kod w internecie

Zrobiłeś to, dobra robota! Podzieliłeś się efektem twojej ciężkiej pracy z ludzkością. Ile osób zabiłeś? Ilu programistów umarło próbując zrozumieć co ten plik robi? Ile ludzkich losów wymazałeś z historii przez to, że nie sprawdziłeś czy twoje dzieło zadziała na dwóch popularnych systemach operacyjnych. Może lepiej napisać co dany moduł robi niż linijkę “DUMNY TWÓRCA KODU Wszystkie prawa zastrzeżone”? Oj tam… kod był trudny do napisania, niech się trochę pomęczą.

 

***

 

Posłowie

Wyszła nowa bezsensowna łatka do Windowsa, instalacja trwała 15 minut. 5048 osób zginęło.

Pomóżmy pracodawcom w odpowiednim przygotowaniu rozmów rekrutacyjnych IT.

Dodaj opinię o interview

Subskrybuj artykuły Bulldogjob

s_o_v_a
See new job offers