Clean code _v2003

Design Patterns in Visual Basic ???

Criteria used in defining clean code The code is mostly read and maintained, not written, so: SHOULD BE LITERATE (Reads like prose) But also: Elegant Unit-testable Has minimal dependencies Clear and minimal API One way rather than many to do things Good error handling Contains minimal duplication Efficient

Meaningful names Functions Comments

Use intention revealing names public List<int[]> getThem() { List<int[]> list1 = new ArrayList<int[]>(); for (int[] x : theList) if (x[0] == 4) list1.add(x); return list1; }

Problems : What kinds of things are in theList? Significance of the zeroth subscript? Significance of the value 4? The most significant problem is actually the IMPLICITY of the code! (the code assumes we know the answers to questions like the ones above).

Refactored version (improved, yet not perfect): public List<int[]> getFlaggedCells() { List<int[]> flaggedCells = new ArrayList<int[]>(); for (int[] cell : gameBoard) if (cell[STATUS_VALUE] == FLAGGED) flaggedCells.add(cell); return flaggedCells; }

Make meaningful distinctions public static void copyChars(char a1[], char a2[]){ for (int i = 0; i < a1.length; i++) { a2[i] = a1[i]; } }

Problems: a1, a2 have no meaningful names. A better version would replace the variable name a1 and a2 with source, respectively destination.

Use pronounceable names class DtaRcrd102 { private Date genymdhms; private Date modymdhms; private final String pszqint = "102"; };

Refactored version: class Customer { private Date generationTimestamp; private Date modificationTimestamp; private final String recordId = "102"; };

Avoid encodings Hungarian notation Was considered important in the Windows C API, for example - everything was an integer handle, long pointer or void pointer. The biggest issue: the compiler did not check the type of a variable.

With modern compilers AND modern programming practices: String strName = “name”; // OBSOLETE, REDUNDANT, CLUTTERS UP THE CODE

Member prefixes public class Part { private String m_dsc; void setName(String description) { m_dsc = description; } }

public class Part { String description; // What's wrong with this? void setDescription(String description) { this.description = description; } }

Avoid mental mapping URL p; // an url used in the application Problems Readers shouldn't have to mentally translate the name p into its meaning. A single-letter name is a poor choice; it’s just a place holder that the reader must mentally map to the actual concept.

One difference between a smart programmer and a PROFESSIONAL programmer is that the professional understands that clarity is king. Professionals write code that others can understand.

The first rule of functions: SMALL Functions should not be 100 lines long. Functions should hardly ever be 20 lines long. Smallness implies that blocks within if statements, else statements, while statements, etc should be one line long, that line should be a function call. functions should not be large enough to hold nested structures.

Therefore the indent level of a function should not be greater than one or two. This rule, of course, makes the functions easier to read and understand.

The second rule of functions: DO ONE THING We write functions to: decompose a larger concept (i.e. the name of the function) into a set of steps at the next level of abstraction.

The third rule of functions: NO MORE THAN THREE ARGUMENTS the ideal number of arguments for a function is zero (niladic). Next comes one (monadic), Followed closely by two (dyadic). Three arguments (triadic) should be avoided where possible. More than three (polyadic) requires very special justification, and then shouldn’t be used anyway.

Output Arguments Arguments are most naturally interpreted as inputs to a function. appendFooter(s); To understand the code, we look at the method signature public void appendFooter(StringBuffer report) { }

Command query separation public boolean set(String attribute, String value) { } This leads to odd statements like this: if (set("username", "unclebob")) { } What does this mean?

Separate the preceding function in two, a command and a query: if (attributeExists("username")) { setAttribute("username", "unclebob"); }

No side effects 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; } }

Problem: the side effect represented by the call to Session.initialize().

Prefer Exceptions to Returning Error Codes 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");

Problems: Returning error codes from command functions is a subtle violation of command query separation: if (deletePage(page) == E_OK) Leads to deeply nested structures.

Possible refactoring try { deletePage(page); registry.deleteReference(page.name); configKeys.deleteKey(page.name.makeKey()); } catch (Exception e) { logger.log(e.getMessage()); }

Comments Do Not Make Up for Bad Code // Check to see if the employee is eligible for full benefits if ((employee.flags && HOURLY_FLAG) && (employee.age > 65)) What about this? if (employee.isEligibleForFullBenefits())

Good comments Informative comments // format matched kk:mm:ss EEE, MMM dd, yyyy Pattern timeMatcher = Pattern.compile( "\\d*:\\d*:\\d* \\w*, \\w* \\d*, \\d*");

Explanation of intent 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. }

Warning of consequences // Don't run unless you // have some time to kill. public void _testWithReallyBigFile() { writeLinesToFile(10000000); } Amplification

Bad comments Unclear explanation public void loadProperties() { try { String propertiesPath = propertiesLocation + "/" + PROPERTIES_FILE; loadedProperties.load(propertiesPath); catch(IOException e) { // No properties files means all defaults are loaded } }

Redundant // 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"); } }

// ContainerBase.java taken from Tomcat public abstract class ContainerBase implements Container, Lifecycle, Pipeline, MBeanRegistration, Serializable { /** * The processor delay for this component. */ protected int backgroundProcessorDelay = -1; /** * The lifecycle event support for this component. */ protected LifecycleSupport lifecycle = new LifecycleSupport(this);

Mandated Comments /** * * @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); }

Noise comments /** * Default constructor. //Oh, really? */ protected AnnualDateRule() { } /** The day of the month. */ private int dayOfMonth; /** * Returns the day of the month. * * @return the day of the month. */ public int getDayOfMonth() { return dayOfMonth; }

Commented out code 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())); Why are those two lines of code commented? Are they important? Were they left as reminders? No one will delete them, they will rot in the code (sometimes for years).

Urmeaza...poze de la manastire...

Clean code _v2003

More Related Content

What's hot

Viewers also liked

Similar to Clean code _v2003

Clean code _v2003