Praktyczne Code Review
ARKADIUSZ KONDAS
Backend Team Leader
@ Da Vinci Studio
Zend Certified Engineer
Code Craftsman
Blogger
Ultra Runner
WeBB MeetUp Autumn
09.12.2015
@ ArkadiuszKondas
itcraftsman.pl
Zend Certified Architect
- Co to jest Code Review ?
- Jak to wygląda w praktyce ?
- Na co zwracać uwagę ?
- Zyski i zagrożenia ?
Agenda:
Co to jest Code Review ?
Co to jest Code Review ?
Henry Oldenburg, sekretarz Royal Society, 1665
Co to jest Code Review ?
Fagan Inspection Process
Michael Fagan
Co to jest Code Review ?
Agile Manifesto
http://agilemanifesto.org
Przeglądanie lekkie:
- do 200-400 wierszy kodu
- do 300-500 wierszy na godzinę
- do 60-90 minut
- nie ma za małych zmian
Jak to wygląda w praktyce ?
Pull/merge request
Pull/merge request
<?php
class ArrayUtils
{
/**
* @param array $array
*
* @return mixed
*/
public function mostRepeatedValue(array $array)
{
}
}
Pull/merge request
class MostRepeatedValueTest extends \PHPUnit_Framework_TestCase
{
public function testIfItReturnMostRepeatedValueInIntegersArray()
{
$array = [1, 3, 4, 5, 1, 3, 1, 7];
$utils = new ArrayUtils();
$mostRepeatedValue = $utils->mostRepeatedValue($array);
$this->assertEquals(1, $mostRepeatedValue);
}
public function testIfItReturnMostRepeatedValueInStringArray()
{
$array = ['Apple', 'Orange', 'Plum', 'Apple', 'Grape', 'Apple'];
$utils = new ArrayUtils();
$mostRepeatedValue = $utils->mostRepeatedValue($array);
$this->assertEquals('Apple', $mostRepeatedValue);
}
}
Pull/merge request
Pull/merge request
public function mostRepeatedValue(array $array)
{
$counts = [];
foreach ($array as $item) {
$item = (string) $item;
if (!isset($counts[$item])) {
$counts[$item] = 1;
} else {
++$counts[$item];
}
}
$mostRepeatedCount = 0;
$mostRepeated = null;
foreach ($counts as $key => $count) {
if ($count > $mostRepeatedCount) {
$mostRepeatedCount = $count;
$mostRepeated = $key;
}
}
return $mostRepeated;
}
Pull/merge request
Pull/merge request
Pull/merge request
public function mostRepeatedValue(array $array)
{
$counts = array_count_values($array);
arsort($counts);
return key($counts);
}
Pull/merge request
Pull/merge request
Pull/merge request
Przydatne wskazówki
- Sprawdź swój pull request
- Nie wywyższaj się
- Nie formułuj bezsensownych żądań
- Szanuj partnerów
- Ucz innych
- Ucz się od innych
Przydatne wskazówki
Na co zwracać uwagę ?
Na co zwracać uwagę ?
Czy rozumiem co przeglądany kod
ma robić ?
function same(aArr, bArr)
{
return [].concat.apply([],aArr).sort().toString()
=== [].concat.apply([],bArr).sort().toString();
}
Na co zwracać uwagę ?
Czy istnieją jakieś oczywiste błędy logiczne w kodzie ?
public function hasCorrectAge($user)
{
return $user->getAge() = 18;
}
// Yoda notation
public function hasCorrectAge($user)
{
return 18 == $user->getAge();
}
Na co zwracać uwagę ?
Czy spełnione zostały wymagania ?
Na co zwracać uwagę ?
Czy istnieją testy automatyczne pokrywające przeglądany kod ?
Na co zwracać uwagę ?
Czy przeglądany kod stosuje się do ustalonego stylu ?
Na co zwracać uwagę ?
Czy sam zrobiłbym coś inaczej ?
Co na tym zyskam ?
Co na tym zyskam ?
Lepsza komunikacja z zespołem
Co na tym zyskam ?
Polepszanie jakości kodu
Co na tym zyskam ?
Łatwiejsze wdrożenie nowego programisty
Co na tym zyskam ?
Wcześniejsze wykrywanie błędów
Co na tym zyskam ?
Dotrzymywanie terminów
Jakie są zagrożenia ?
Jakie są zagrożenia ?
Brak widocznego kontekstu zmian
Jakie są zagrożenia ?
Osoba sprawdzająca z poza projektu, brak wiedzy na temat domeny
/**
* @throws EmptyVesselException
*/
public function emptyTheContent()
{
if ($this->isEmpty()) {
throw new EmptyVesselException();
}
$this->filledWith = null;
$this->ingredients = [];
$this->currentCapacity = new Capacity(0);
}
Jakie są zagrożenia ?
Różny poziom osób sprawdzających kod
Jakie są zagrożenia ?
Mniejsze poczucie odpowiedzialności
"przecież mi zaakceptowałeś ten kodzik"
Opór przed wdrożęniem
Pytania ?
Dziękuję za uwagę
@ ArkadiuszKondas
itcraftsman.pl
Praktyczne Code Review
By Arkadiusz Kondas
Praktyczne Code Review
Code Review czyli inspekcja kodu. Chcę odpowiedzieć na pytania: co to takiego code review ? dlaczego warto robić code review ? oraz jak zrobić to dobrze ?
- 1,603