Transcript pptx

Czysty Kod
Wprowadzenie
Przygotował Adam Zawadzki
Agenda
•
•
•
•
•
•
•
•
•
Dlaczego warto dbać o na jakość kodu?
Dług techniczny
Nazewnictwo
Metody
Komentarze
Formatowanie
Obsługa wyjątków
Klasy
Zewnętrzne biblioteki
Dlaczego warto dbać o na jakość kodu?
• Jakosć kodu ma wpływ na prodkuktywność
• Jakość kodu wpływa na nasze nastawienie do pracy
Dług
techniczny
Czysty Kod
Nazewnictwo
Używanie innego języka niż angielski w nazewnictwie
Używaj nazw odzwierciedlających Twoje intencje
public List<int[]> getThem() {
List<int[]> list1 = new ArrayList<int[]>();
for (int[] x : theList) {
if (x[0] == 4) list1.add(x);
}
return list1;
}
Używaj nazw odzwierciedlających Twoje intencje
public List<int[]> getFlaggedCells() {
List<int[]> flaggedCells = new ArrayList<int[]>();
for (int[] cell : gameBoard)
if (cell[STATUS_VALUE] == FLAGGED) flaggedCells.add(cell);
return flaggedCells;
}
public List<Cell> getFlaggedCells() {
List<Cell> flaggedCells = new ArrayList<Cell>();
for (Cell cell : gameBoard)
if (cell.isFlagged()) flaggedCells.add(cell);
return flaggedCells;
}
Unikaj dezinformacji w nazewnictwie
•
•
•
•
•
String hp;
Integer aix;
accountList;
XYZControllerForEfficientHandlingOfStrings
XYZControllerForEfficientStorageOfStrings
Znaczące rozróżnienia
public static void copyChars(char a1[], char a2[]) {
for (int i = 0; i < a1.length; i++) {
a2[i] = a1[i];
}
}
Używaj nazw które da się wymówić
class DtaRcrd102 {
private Date genymdhms;
private Date modymdhms;
private final String pszqint = "102";
/* ... */
};
class Customer {
private Date generationTimestamp;
private Date modificationTimestamp;;
private final String recordId = "102";
/* ... */
};
Unikaj prefixów
public class Part {
private String m_dsc; // The textual description
void setName(String name) {
m_dsc = name;
}
}
public class Part {
String description;
void setDescription(String description) {
this.description = description;
}
Nazwy klas
• Nazwa klasy powinna być rzeczownikiem: Customer, WikiPage, Account, AddressParser
• Unikaj nazw takich jak Manager, Processor, Data, or Info
• Nie używaj czaswoników w nazwach klas
Nazwy metod
• Nazwy metod wykonujacych operacje powinnt być czasownikami: deletePage,
savePersistable, addToList.
• Metody będąco akcesorami, mutatorami lub predykatami powinny być nazywane
zgodnie z konwencją java beans: get, set, is.
• Nie próbuj demnonstrować swojego poczucia humoru w kodzie. Jest to
nieakceptowalne.
Używaj zawsze tego samego słowa dla danej koncepcji
• Jeżeli w danym projekcie dla akcesorów przyjęto nadawać nazwy z jednym
przedrostkiem(get, fetch, retrieve) trzymaj się tej konwencji.
• Unikajmy mieszania koncepcji:
•
Używając wzorca MVC nie nazywajmy klasy kontrolera: CustomerManager
•
Nie zastępujmy końcówki Manager używając Driver
• Należy pamiętać że spójny słownik to ważny element czytelnego kodu.
Używaj nazw w domenie rozwiązania
• Czytającym Twój kod będzie inny programista
• Rozwiązania techniczne opisuj przy pomocy terminów technicznych: AccountVisitor,
CustomerFactory, JobQueue.
Używaj nazw w domenie problemu
Jeżeli projektujemy system obsługujący lotnisko to zamiast używać słowa:
• vehicle użyjmy słowa airplane,
• zamiast zlepku aSubstanceWhichKeepsAirplaneFlying użyjmy fuel
• Keep it simple.
Czysty Kod
Metody
Funkcje muszą być małe
• Im dłuższa funkcja tym mniej czytelna
public static String renderPageWithSetupsAndTeardowns(
PageData pageData, boolean isSuite) throws Exception {
if (isTestPage(pageData))
includeSetupAndTeardownPages(pageData, isSuite);
return pageData.getHtml();
}
Jedna funkcja robi wyłącznie jedną rzecz w danym kontekście
Dana funkcja powinna wykonywać operacje na jednym poziomie
abstrakcji
public void renderAddressDataTableForPerson(Person p)
AddressData[] allAddressData = getAllAddressData();
AddressData personAddresData = getPersonAddresData(p, addressData);
rednerAddresDataTable(personAddresData);
}
private AddressData[] getAllAddressData(){
connectDatabase();
AddressData[] addressData = fetchAllAddressData();
disconnectDatabase();
return addressData;
}
private void connectDatabase(){
/* Connect database. */
}
private void fetchAllAddressData(){
/* Fetch all address data from database. */
}
private void disconnectDatabase(){
/* Disconnect database. */
}
private AddressData getPersonAddresData(Person p, AddressData[] addressData){
/* Iterate over array and return address of selected person. */
}
private AddressData rednerAddressDataTable(AddressData addressData){
/* Render address table. */
}
Zasada Stepdown
public void doSomething(String input){
try {
doSomethingWell(input);
} catch(Exception ex) {
LOGGER.error(ex.getMessage(), ex);
}
}
private void doSomethingWell(String input){
/*
* Do something well.
*/
doSomethingVeryWell(input);
}
private void doSomethingVeryWell(String input){
/*
* Do something very well.
*/
}
Używaj opisowych nazw funkcji
• Nazwy muszą być jednoznaczne w danym kontekscie
• Nazwy muszą być spójne między sobą
Argumenty funkcji
•
•
•
•
•
•
•
Funkcje bezargumentowe są świetne: record.remove();
Funkcje z jednym argumentem są bardzo dobre: file.save(„output”);
Funkcje z dwoma argumentami są dobre: file.open(„input”, „r”);
Funkcje z trzema są akceptowalne: assertEquals(message, expected, actual);
Funkcje z więcej niż trzema argumentami są wstrętne i należy ich unikać.
Funkje z argumentami flagowymi powinny być unikane. Łamią zasadę SPR.
Listy argumentów:
•
void monad(Integer... args);
•
void dyad(String name, Integer... args);
•
void triad(String name, int count, Integer... args);
Efekty uboczne
public class UserValidator {
private Cryptographer cryptographer;
public boolean checkPassword(String userName, String password) {
User user = UserGateway.findByName(userName);
if (user != User.NULL) {
String codedPhrase = user.getPhraseEncodedByPassword();
String phrase = cryptographer.decrypt(codedPhrase, password);
if ("Valid Password".equals(phrase)) {
Session.initialize();
return true;
}
}
return false;
}
}
Argumenty wyjściowe
• appendFooter(s);
• report.appendFooter();
Separuj komendy od zapytań
public boolean set(String attribute, String value);
if (set("username", "unclebob")){ /* ... */ }
if (attributeExists("username")) {
setAttribute("username", "unclebob");
//...
}
Używaj wyjątków, nie kodów błędów.
if (deletePage(page) == E_OK) {
if (registry.deleteReference(page.name) == E_OK) {
if (configKeys.deleteKey(page.name.makeKey()) == E_OK){
logger.log("page deleted");
} else {
logger.log("configKey not deleted");
}
} else {
logger.log("deleteReference from registry failed");
}
} else {
logger.log("delete failed");
return E_ERROR;
}
Używaj wyjątków, nie kodów błędów.
try {
deletePage(page);
registry.deleteReference(page.name);
configKeys.deleteKey(page.name.makeKey());
}catch (Exception e) {
logger.log(e.getMessage());
}
Bloki try/catch
public void delete(Page page) {
try {
deletePageAndAllReferences(page);
}catch (Exception e) {
logError(e);
}
}
private void deletePageAndAllReferences(Page page) throws Exception {
deletePage(page);
registry.deleteReference(page.name);
configKeys.deleteKey(page.name.makeKey());
}
private void logError(Exception e) {
logger.log(e.getMessage());
}
Dodatkowo
• Wydzielaj obsługę błędów.
• Nie duplikuj kodu. Jeżeli coś powtarza się dwa razy można z tego zrobić oddzielną
funkcję.
• Refaktoryzuj
Czysty Kod
Komentarze. Nie sprawiają, że zły kod staje się lepszy.
Wyjaśniaj za pomocą kodu
// Check to see if the employee is eligible for full benefits
if ((employee.flags & HOURLY_FLAG) && (employee.age > 65))
if (employee.isEligibleForFullBenefits())
Dobre komentarze
• Licencje
• Informacje:
// format matched kk:mm:ss EEE, MMM dd, yyyy
Pattern timeMatcher = Pattern.compile(„\\d*:\\d*:\\d* \\w*, \\w* \\d*, \\d*");
• Wyjaśnienie intencji:
public int compareTo(Object o)
{
if(o instanceof WikiPagePath)
{
WikiPagePath p = (WikiPagePath) o;
String compressedName = StringUtil.join(names, "");
String compressedArgumentName = StringUtil.join(p.names, "");
return compressedName.compareTo(compressedArgumentName);
}
return 1; // we are greater because we are the right type.
}
Dobre komentarze
•
Wyjaśnienia
public void testCompareTo() throws Exception
{
WikiPagePath a = PathParser.parse("PageA");
WikiPagePath ab = PathParser.parse("PageA.PageB");
WikiPagePath b = PathParser.parse("PageB");
WikiPagePath aa = PathParser.parse("PageA.PageA");
WikiPagePath bb = PathParser.parse("PageB.PageB");
WikiPagePath ba = PathParser.parse("PageB.PageA");
assertTrue(a.compareTo(a) == 0);
// a == a
assertTrue(a.compareTo(b) != 0);
// a != b
assertTrue(ab.compareTo(ab) == 0); // ab == ab
assertTrue(a.compareTo(b) == -1);
// a < b
assertTrue(aa.compareTo(ab) == -1); // aa < ab
assertTrue(ba.compareTo(bb) == -1); // ba < bb
assertTrue(b.compareTo(a) == 1);
// b > a
assertTrue(ab.compareTo(aa) == 1); // ab > aa
assertTrue(bb.compareTo(ba) == 1); // bb > ba
}
Dobre komentarze
• Ostrzeżenia o konsekwencjach
// Don't run unless you
// have some time to kill.
public void _testWithReallyBigFile()
{
writeLinesToFile(10000000);
response.setBody(testFile);
response.readyToSend(this);
String responseString = output.toString();
assertSubString("Content-Length: 1000000000", responseString);
assertTrue(bytesSent > 1000000000);
}
Dobre komentarze
• TODO’s
//TODO-MdM these are not needed
// We expect this to go away when we do the checkout model
protected VersionInfo makeVersion() throws Exception
{
return null;
}
Dobre komentarze
• Wzmocnienia
String listItemContent = match.group(3).trim();
// the trim is real important. It removes the starting
// spaces that could cause the item to be recognized
// as another list.
new ListItemWidget(this, listItemContent, this.level + 1);
return buildList(text.substring(match.end()));
• Java Docs w publicznym API
Złe komentarze
• Mamrotanie
public void loadProperties()
{
try
{
String propertiesPath = propertiesLocation + "/" + PROPERTIES_FILE;
FileInputStream propertiesStream = new FileInputStream(propertiesPath);
loadedProperties.load(propertiesStream);
}catch(IOException e){
// No properties files means all defaults are loaded
}
}
Złe komentarze
• Zbędne komentarze
// Utility method that returns when this.closed is true. Throws an exception
// if the timeout is reached.
public synchronized void waitForClose(final long timeoutMillis) throws Exception {
if(!closed){
wait(timeoutMillis);
if(!closed)
throw new Exception("MockResponseSender could not be closed");
}
}
}
Złe komentarze
•
Zbędne komentarze
public abstract class ContainerBase implements Container, Lifecycle, Pipeline, MBeanRegistration, Serializable {
/**
* The processor delay for this component.
*/
protected int backgroundProcessorDelay = -1;
/**
* The container event listeners for this Container.
*/
protected ArrayList listeners = new ArrayList();
/**
* The Loader implementation with which this Container is
* associated.
*/
protected Loader loader = null;
/**
* The Logger implementation with which this Container is
* associated.
*/
protected Log logger = null;
}
Złe komentarze
• Obowiązkowe komentarze
/**
*
* @param title The title of the CD
* @param author The author of the CD
* @param tracks The number of tracks on the CD
* @param durationInMinutes The duration of the CD in minutes
*/
public void addCD(String title, String author,int tracks, int durationInMinutes) {
CD cd = new CD();
cd.title = title;
cd.author = author;
cd.tracks = tracks;
cd.duration = duration;
cdList.add(cd);
}
Złe komentarze
• Dzienniki zmian
/*
* Changes (from 11-Oct-2001)
* -------------------------* 11-Oct-2001 : Re-organised the class and moved it to new package
* com.jrefinery.date (DG);
* 05-Nov-2001 : Added a getDescription() method, and eliminated NotableDate
* class (DG);
* 12-Nov-2001 : IBD requires setDescription() method, now that NotableDate
* class is gone (DG); Changed getPreviousDayOfWeek(),
* getFollowingDayOfWeek() and getNearestDayOfWeek() to correct bugs (DG);
* 05-Dec-2001 : Fixed bug in SpreadsheetDate class (DG);
* 29-May-2002 : Moved the month constants into a separate interface (MonthConstants)
(DG);
*/
Złe komentarze
• Komentarze robiące zbędny szum
Złe komentarze
• Przerażający szum JavaDoc
/** The name. */
private String name;
/** The version. */
private String version;
/** The licenceName. */
private String licenceName;
/** The version. */
private String info;
private String name;
private String version;
private String licenceName;
private String info;
Złe komentarze
• Markery pozycji : /// Actions //////////////////////////////////
• Komentarze zamykające:
try{
while ((line = in.readLine()) != null) {
lineCount++;
charCount += line.length();
String words[] = line.split("\\W");
wordCount += words.length;
} //while
System.out.println("wordCount = " + wordCount);
System.out.println("lineCount = " + lineCount);
System.out.println("charCount = " + charCount);
} // try
catch (IOException e) {
System.err.println("Error:" + e.getMessage());
} //catch
Złe komentarze
• Komentarze na temat autora: // By [email protected]
• Wykomentowany kod:
InputStreamResponse response = new InputStreamResponse();
response.setBody(formatter.getResultStream(), formatter.getByteCount());
// InputStream resultsStream = formatter.getResultStream();
// StreamReader reader = new StreamReader(resultsStream);
// response.setContent(reader.read(formatter.getByteCount()));
Złe komentarze
• Komentarze HTML:
Złe komentarze
• Informacje nie dotyczące danego fragmentu kodu:
/**
* Port on which fitnesse would run. Defaults to <b>8082</b>.
*
* @param fitnessePort
*/
public void setFitnessePort(int fitnessePort)
{
this.fitnessePort = fitnessePort;
}
• Nie jasne komentarze:
/*
* start with an array that is big enough to hold all the pixels
* (plus filter bytes), and an extra 200 bytes for header info
*/
this.pngBytes = new byte[((this.width + 1) * this.height * 3) + 200];
Złe komentarze
• JavaDoc’s w niepublicznym kodzie:
/**
* Method foo which is doing something;
* @return int
*/
private int foo(String s);
Czysty Kod
Formatowanie
Ogólne zasady formatowania
• Na danym projekcie wszyscy powinni się stosować do tych samych zasad formatowania
• Zawsze należy usuwać trailing spaces z kodu.
Formatowanie pionowe
• Im plik jest krótszy tm bardziej czytelny
• Kod w pliku powinien być tak ułożony, aby wspierać czytanie go z góry do dołu.
• Przestrzenie w pionie powinny rozdzielać różne koncepcje:
Formatowanie pionowe
• Zmienne powinny być deklarowane jak najbliżej miejsca w którym będą używane
• Funkcje zależne powinny być ułożone obok siebie
• Funkcje obejmujące jedna koncepcję powinny być ułożone blisko siebie
Formatowanie poziome
• Unikajmy ściskania kodu w poziomie
• Wstawiajmy spacje żeby zwiększyć czytelność kodu:
public class Quadratic {
public static double root1(double a, double b, double c) {
double determinant = determinant(a, b, c);
return (-b + Math.sqrt(determinant)) / (2*a);
}
public static double root2(int a, int b, int c) {
double determinant = determinant(a, b, c);
return (-b - Math.sqrt(determinant)) / (2*a);
}
private static double determinant(double a, double b, double c) {
return b*b - 4*a*c;
}
}
Formatowanie poziome
• Unikajmy wyrównywania w poziomie:
Formatowanie poziome
• Wcięcia w kodzie zwiększają jego czytelność:
• Długość linii nie powinna przekraczać około 120 znaków
Czysty Kod
Obsługa wyjątków
Jedna funkcja = jeden blok try/catch
public List<RecordedGrip> retrieveSection(String sectionName) {
try {
FileInputStream stream = new FileInputStream(sectionName)
} catch (Exception e) {
throw new StorageException("retrieval error", e);
}
return new ArrayList<RecordedGrip>();
}
Z wyjątkiem dostarczaj kontekst
Nie zwracaj NULL
public void registerItem(Item item) {
if (item != null) {
ItemRegistry registry = peristentStore.getItemRegistry();
if (registry != null) {
Item existing = registry.getItem(item.getID());
if (existing.getBillingPeriod().hasRetailOwner()) {
existing.register(item);
}
}
}
}
Nie przekazuj NULL
public double xProjection(Point p1, Point p2) {
if (p1 == null || p2 == null) {
throw InvalidArgumentException(
"Invalid argument for MetricsCalculator.xProjection");
}
return (p2.x – p1.x) * 1.5;
}
Czysty Kod
Praca z zewnętrznymi bibliotekami
Zewnętrzne biblioteki
• Opakowuj zewnętrzny kod
• Napisz testy dla wykorzystywanych zewnętrznych funkcjonalności
Czysty Kod
Klasy
Organizacja klasy
• Zgodnie ze standardami, na samym początku zawsze umieszczamy pola klasy.
• Klasa powinna udostępniać na zewnątrz tylko funkcje odpowiedzialne za realizację
konkretnych zadań. Nie istotnie jest to jak je realizuje. Mechanika powinna być ukryta.
Klasy powinny być małe
• The Single Responsibility Principle. Nigdy nie powinien istnieć więcej niż jeden powód do
zmiany klasy.
• Spójność. Klasy powinny mieć małą liczbę zmiennych instancyjnych. Każda metoda klasy
powinna manipulować przynajmniej jedną zmienną.
Bibliografia
Clean Code: A Handbook of Agile Software Craftsmanship
By Robert C. Martin