Git Product home page Git Product logo

clean-code's People

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar

clean-code's Issues

Functions - Do One Thing

FUNCTIONS SHOULD DO ONE THING. THEY SHOULD DO IT WELL. THEY SHOULD DO IT ONLY.

public static String renderPageWithSetupsAndTeardowns(
    PageData pageData, boolean isSuite
) throws Exception {
    boolean isTestPage = pageData.hasAttribute("Test");
    if (isTestPage) {
      WikiPage testPage = pageData.getWikiPage();
      StringBuffer newPageContent = new StringBuffer();
      includeSetupPages(testPage, newPageContent, isSuite);
      newPageContent.append(pageData.getContent());
      includeTeardownPages(testPage, newPageContent, isSuite);
      pageData.setContent(newPageContent.toString());
    }
    return pageData.getHtml();
}
--------------------------------------------------------------------
public static String renderPageWithSetupsAndTeardowns(
    PageData pageData, boolean isSuite) throws Exception {
      if (isTestPage(pageData))
        includeSetupAndTeardownPages(pageData, isSuite);
      return pageData.getHtml();
    }
  1. Determining whether the page is a test page.
  2. If so, including setups and teardowns.
  3. Rendering the page in HTML.
  • So which is it? Is the function doing one thing or three things? Notice that the three
    steps of the function are one level of abstraction below the stated name of the function. We
    can describe the function by describing it as a brief TO paragraph(TO RenderPageWithSetupsAndTeardowns, we check to see whether the page is a test page and if so, we include the setups and teardowns. In either case we render the page in HTML.)
  • If a function does only those steps that are one level below the stated name of the
    function, then the function is doing one thing

Boundaries - Using Third-Party Code

  • If our application needs a Map of Sensors, you might find the sensors set up like this:
    java Map sensors = new HashMap();
  • Then, when some other part of the code needs to access the sensor, you see this code:
    java Sensor s = (Sensor)sensors.get(sensorId );
  • This works, but it’s not clean code. Also, this code does not tell its story as well as it
    could.
  • The readability of this code can be greatly improved by using generics, as shown
    below:
Map<Sensor> sensors = new HashMap<Sensor>();
...
Sensor s = sensors.get(sensorId );```

Functions - Small

  1. The first rule of functions is that they should be small.
  2. The second rule of functions is that they should be smaller than that.
  3. A function should be no bigger than a screen-full. 24 lines by 80 columns
  4. Nowadays with a cranked-down font and a nice big monitor, you can fit 150 characters on a line and a 100 lines or more on a screen

Formatting - Vertical Formatting

  • None are over 500 lines and most of those files are less than 200 lines.
  • Small files are usually easier to understand than large files are.

image

Meaningful Names - Use Searchable Names

Compare:

    for (int j=0; j<34; j++) {
      s += (t[j]*4)/5;
    }

To:

    int realDaysPerIdealDay = 4;
    const int WORK_DAYS_PER_WEEK = 5;
    int sum = 0;
    for (int j=0; j < NUMBER_OF_TASKS; j++) {
      int realTaskDays = taskEstimate[j] * realDaysPerIdealDay;
      int realTaskWeeks = (realdays / WORK_DAYS_PER_WEEK);
      sum += realTaskWeeks;
   }

Functions - Prefer Exceptions to Returning Error Codes

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());
}
  • In the above, the delete function is all about error processing. It is easy to understand
    and then ignore. The deletePageAndAllReferences function is all about the processes of
    fully deleting a page. Error handling can be ignored. This provides a nice separation that
    makes the code easier to understand and modify.

Comments - Explain Yourself in Code

Don’t comment bad code—rewrite it!

  • Which would you rather see? This
//Check to see if the employee is eligible for full benefits
if ((employee.flags & HOURLY_FLAG) && (employee.age > 65))

Or this?

if (employee.isEligibleForFullBenefits())

Formatting - The Newspaper Metaphor

  • Think of a well-written newspaper article. You read it vertically. At the top you expect a headline that will tell you what the story is about and allows you to decide whether it is something you want to read. The first paragraph gives you a synopsis of the whole story, hiding all the details while giving you the broad-brush concepts. As you continue downward, the details increase until you have all the dates, names, quotes, claims, and other minutia.
  • We would like a source file to be like a newspaper article. The name should be simple but explanatory. The name, by itself, should be sufficient to tell us whether we are in the right module or not. The topmost parts of the source file should provide the high-level concepts and algorithms. Detail should increase as we move downward, until at the end we find the lowest level functions and details in the source file. A newspaper is composed of many articles; most are very small. Some are a bit larger. Very few contain as much text as a page can hold. This makes the newspaper usable. If the newspaper were just one long story containing a disorganized agglomeration of facts, dates, and names, then we simply would not read it.

Functions - Argument Objects

  • When a function seems to need more than two or three arguments, it is likely that some of those arguments ought to be wrapped into a class of their own. Consider, for example, the difference between the two following declarations:
Circle makeCircle(double x, double y, double radius);
Circle makeCircle(Point center, double radius);
  • Reducing the number of arguments by creating objects out of them may seem like cheating, but it’s not. When groups of variables are passed together, the way x and y are in the example above, they are likely part of a concept that deserves a name of its own.

Meaningful Names - Make Meaningful Distinctions

  • Because you can’t use the same name to refer to two different things in the same scope, you might be tempted to change one name in an arbitrary way. Sometimes this is done by misspelling one, leading to the surprising situation where correcting spelling errors leads to an inability to compile:
  • Example: class -> klass
  • Number-series naming (a1, a2, .. aN) is the opposite of intentional naming. Such names are not disinformative—they are noninformative; they provide no clue to the author’s intention. Consider:
public static void copyChars(char a1[], char a2[]) {
    for (int i = 0; i < a1.length; i++) {
      a2[i] = a1[i];
    }
}

=> This function reads much better when source and destination are used for the argument

names.

getActiveAccount();
getActiveAccounts();
getActiveAccountInfo();

How are the programmers in this project supposed to know which of these functions to call?

Meaningful Names - Avoid Encodings

We have enough encodings to deal with without adding more to our burden. Encoding type or scope information into names simply adds an extra burden of deciphering. It hardly seems reasonable to require each new employee to learn yet another encoding “language” in addition to learning the (usually considerable) body of code that they’ll be working in. It is an unnecessary mental burden when trying to solve a problem. Encoded names are seldom pronounceable and are easy to mis-type.

Meaningful Names - Use Intention-Revealing Names

Use Intention-Revealing Names

  • The name of a variable, function, or class, should answer all the big questions. It
    should tell you why it exists, what it does, and how it is used. If a name requires a comment,
    then the name does not reveal its intent
int d; // elapsed time in days
  • The name d reveals nothing. It does not evoke a sense of elapsed time, nor of days. We
    should choose a name that specifies what is being measured and the unit of that measurement:
int elapsedTimeInDays;
int daysSinceCreation;
int daysSinceModification;
int fileAgeInDays;
  • Choosing names that reveal intent can make it much easier to understand and change
    code. What is the purpose of this code?
public List<int[]> getThem() {
   List<int[]> list1 = new ArrayList<int[]>();
   for (int[] x : theList)
     if (x[0] == 4)
       list1.add(x);
   return list1;
}
  1. What kinds of things are in theList?
  2. What is the significance of the zeroth subscript of an item in theList?
  3. What is the significance of the value 4?
  4. How would I use the list being returned?

==>Should be:

public List<int[]> getFlaggedCells() {
    List<int[]> flaggedCells = new ArrayList<int[]>();
    for (int[] cell : gameBoard)
      if (cell[STATUS_VALUE] == FLAGGED)
        flaggedCells.add(cell);
    return flaggedCells;
}

==>Better:

public List<Cell> getFlaggedCells() {
    List<Cell> flaggedCells = new ArrayList<Cell>();
      for (Cell cell : gameBoard)
        if (cell.isFlagged())
          flaggedCells.add(cell);
    return flaggedCells;
}

Meaningful Names - Use Pronounceable Names

  • The variables explained to them
    Compare:
class DtaRcrd102 {
    private Date genymdhms;
    private Date modymdhms;
   private final String pszqint = "102";
    /* ... */
};

to

class Customer {
    private Date generationTimestamp;
    private Date modificationTimestamp;;
    private final String recordId = "102";
    /* ... */
};

Functions - Command Query Separation

  • Functions should either do something or answer something, but not both
    Example:
public boolean set(String attribute, String value); 
if (set("username", "unclebob"))...
  • Separate the command from the query so that the ambiguity cannot occur.
if (attributeExists("username")) {
    setAttribute("username", "unclebob");
    ...
}

Functions - Flag Arguments

  • Flag arguments are ugly. Passing a boolean into a function is a truly terrible practice. It
    immediately complicates the signature of the method, loudly proclaiming that this function
    does more than one thing. It does one thing if the flag is true and another if the flag is false!
  • Method call render(true) is just plain confusing to a poor reader
  • Method
render(boolean isSuite)

Should have split the function into two:

renderForSuite() 

and

renderForSingleTest().

Unit Tests - The Three Laws of Test Driven Development

By now everyone knows that TDD asks us to write unit tests first, before we write production code. But that rule is just the tip of the iceberg. Consider the following three laws:

  1. First Law You may not write production code until you have written a failing unit test.
  2. Second Law You may not write more of a unit test than is sufficient to fail, and not compiling
    is failing.
  3. Third Law You may not write more production code than is sufficient to pass the currently
    failing test.

Meaningful Names - Pick One Word per Concept

  • For instance, it’s confusing to have fetch, retrieve, and get as equivalent methods of different classes
  • A consistent lexicon is a great boon to the programmers who must use your code.

Meaningful Names - Member Prefixes

You also don’t need to prefix member variables with m_ anymore

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;
    }
}

People quickly learn to ignore the prefix (or suffix) to see the meaningful part of the name

Meaningful Names - Method Names

  • Methods should have verb or verb phrase names like postPayment, deletePage, or save. Accessors, mutators, and predicates should be named for their value and prefixed with get, set, and is according to the javabean standard.
string name = employee.getName();
customer.setName("mike");
if (paycheck.isPosted())...
  • When constructors are overloaded, use static factory methods with names that describe the arguments. For example,
Complex fulcrumPoint = Complex.FromRealNumber(23.0);

is generally better than

Complex fulcrumPoint = new Complex(23.0);

Consider enforcing their use by making the corresponding constructors private.

Error Handling

  1. Use Exceptions Rather Than Return Codes
  2. Write Your Try-Catch-Finally Statement First
  3. Use Unchecked Exceptions
  4. Provide Context with Exceptions
  5. Define Exception Classes in Terms of a Caller’s Needs
  6. Don’t Return Null, if null return an empty collection
public List<Employee> getEmployees() {
    if( .. there are no employees .. )
      return Collections.emptyList();
}
  1. Don’t Pass Null
public double xProjection(Point p1, Point p2) {
    if (p1 == null || p2 == null) {
      throw InvalidArgumentException(
        "Invalid argument for MetricsCalculator.xProjection");
    }
    return (p2.xp1.x) * 1.5;
    }
}

Functions - Function Arguments

  1. The ideal number of arguments for a function is zero (niladic).
  2. Next comes one (monadic), followed closely by two (dyadic).
  3. Three arguments (triadic) should be avoided where possible.
  4. More than three (polyadic) requires very special justification—and then shouldn’t be used anyway.

Functions - The Error.java Dependency Magnet

public enum Error {
    OK,
    INVALID,
    NO_SUCH,
    LOCKED,
    OUT_OF_RESOURCES,
    WAITING_FOR_EVENT;
}
  • Classes like this are a dependency magnet; many other classes must import and use them. Thus, when the Error enum changes, all those other classes need to be recompiled and redeployed. This puts a negative pressure on the Error class. Programmers don’t want to add new errors because then they have to rebuild and redeploy everything. So they reuse old error codes instead of adding new ones.
  • When you use exceptions rather than error codes, then new exceptions are derivatives of the exception class. They can be added without forcing any recompilation or redeployment.

Functions - Common Monadic Forms

Try to avoid any monadic functions that don’t follow these forms

For example,

void includeSetupPageInto(StringBuffer pageText). 

Using an output argument instead of a return value for a transformation is confusing. If a function is going to transform its input argument, the transformation should appear as the return value. Indeed,

StringBuffer transform(StringBuffer in) 

is better than void transform-(StringBuffer out), even if the implementation in the first case simply returns the input argument. At least it still follows the form of a transformation.

Objects and Data Structures - The Law of Demeter

  • The method should not invoke methods on objects that are returned by any of the allowed functions. In other words, talk to friends, not to strangers
final String outputDir = ctxt.getOptions().getScratchDir().getAbsolutePath();
  • It is usually best to split them up as follows:
Options opts = ctxt.getOptions();
File scratchDir = opts.getScratchDir();
final String outputDir = scratchDir.getAbsolutePath();

Test Driven Development - Idea

27820318-5248fec2-60c8-11e7-92c0-2dd1f133ea87

TDD life-cycle

  1. Write the test
  2. Run the test (there is no implementation code, test does not pass)
  3. Write just enough implementation code to make the test pass
  4. Run all tests (tests pass)
  5. Refactor
  6. Repeat

Test Driven Development

Test Driven Development is the practice of writing a test for a piece of required functionality, before writing any implementation code. This test should fail when first run, and then, you write the code to get it to pass. It doesn't have to be the most perfect code, just so long as the test passes. Once it does, you can then safely refactor your code.

TDD is a discipline, and as such you must try your best to not be tempted to write tests after you've written code. Pressure can come from clients or employers to delay or not bother with the writing the tests at all. Firstly, they need to understand the benefits of tests, and then the additional benefits of TDD on top so you can secure that time to practice TDD correctly.

If your application has tests that were written after the code that implements them, that means TDD wasn't followed. It does mean however that your project has test coverage, which is a good thing that has many benefits (and is definitely better than not having any tests), but practising TDD brings additional benefits on top of those. Often, articles on the internet claim to write about those additional benefits but instead seem to end up focusing on the benefits of tests in general. This post will try and focus strictly on the benefits of TDD.

You shouldn't assume either that TDD doesn't mix with BDD. The key is writing the tests before the code. When writing feature specs that define the behaviour of something, if you're writing those specifications before implementing them, you're TDD'ing too.

At Made, we use TDD when writing our feature specs. We use tools most commonly used for unit tests for our feature tests too, such as RSpec with Capybara helpers, rather than things like Cucumber, which are often associated with BDD (that said, you can still use Cucumber for feature specs and TDD). In all of the time that we've been using TDD, these are the biggest benefits we've noticed along the way:

Benefits

1: Acceptance Criteria
When writing some new code, you usually have a list of features that are required, or acceptance criteria that needs to be met. You can use either of these as a means to know what you need to test and then, once you've got that list in the form of test code, you can rest safely in the knowledge that you haven't missed any work.

2: Focus
You're more productive while coding, and TDD helps keep that productivity high by narrowing your focus. You'll write one failing test, and focus solely on that to get it passing. It forces you to think about smaller chunks of functionality at a time rather than the application as a whole, and you can then incrementally build on a passing test, rather than trying to tackle the bigger picture from the get-go, which will probably result in more bugs, and therefore a longer development time.

3: Interfaces
Because you're writing a test for a single piece of functionality, writing a test first means you have to think about the public interface that other code in your application needs to integrate with. You don't think about the private methods or inner workings of what you're about to work on. From the perspective of the test, you're only writing method calls to test the public methods. This means that code will read well and make more sense.

4: Tidier Code
Continuing on from the point above, your tests are only interfacing with public methods, so you have a much better idea of what can be made private, meaning you don't accidentally expose methods that don't need to be public. If you weren't TDD'ing, and you made a method public, you'd then possibly have to support that in the future, meaning you've created extra work for yourself over a method that was only intended to be used internally in a class.

5: Dependencies
Will your new code have any dependencies? When writing your tests, you'll be able to mock these out without really worrying about what they are doing behind the scenes, which lets you focus on the logic within the class you're writing. An additional benefit is that the dependencies you mock would potentially be faster when running the tests, and not bring additional dependencies to your test suite, in the form of filesystems, networks, databases etc.

6: Safer Refactoring
Once you've got a test passing, it's then safe to refactor it, secure in the knowledge that the test cases will have your back. If you're having to work with legacy code, or code that someone else has written, and no tests have been written, you can still practice TDD. You needn't have authored the original code in order for you to TDD. Rather than thinking you can only TDD code that you have written, think of it more as you can only TDD any code you are about to write. So if you inherit someone else's untested code, before you start work, write a test that covers as much as you can. That puts you in a better position to refactor, or even to add new functionality to that code, whilst being confident that you won't break anything.

7: Fewer Bugs
TDD results in more tests, which can often result in longer test run times. However, with better code coverage, you save time down the line that would be spent fixing bugs that have popped up and need time to figure out. This is not to say that you might be able to think of every test case, but if a bug does come up, you can still write a test first before attempting to fix the problem, to ensure that the bug won't come up again. This also helps define what the bug actually is, as you always need reproducible steps.

8: Increasing Returns
The cost to TDD is higher at first, when compared to not writing any tests, though projects that don't have tests written first usually end up costing more. This stems from them not having decent test code coverage, or any at all, making them more susceptible to bugs and issues, which means more time is spent in the long run fixing those. More time equals more money, which makes the project more expensive overall. Not only does TDD save time on fixing bugs, it also means that the cost to change functionality is less, because the tests act as a safety net that ensure your changes won't break existing functionality.

9: Living Documentation
Tests can serve as documentation to a developer. If you're unsure of how a class or library works, go and have a read through the tests. With TDD, tests usually get written for different scenarios, one of which is probably how you want to use the class. So you can see the expected inputs a method requires and what you can expect as outcome, all based on the assertions made in the test.

Meaningful Names - Avoid Disinformation

  • Avoid words whose entrenched meanings vary from our intended meaning:
    For example, hp, aix, and sco would be poor variable names because they are the names of Unix platforms or variants.
  • Do not refer to a grouping of accounts as an accountList unless it’s actually a List. If the container holding the accounts is not actually a List, it may lead to false conclusions. So accountGroup or bunchOfAccounts or just plain accounts would be better.
  • A truly awful example of disinformative names would be the use of lower-case L or
    uppercase O as variable names, especially in combination:
    int a = l;
    if ( O == l )
      a = O1;
    else
      l = 01;

Functions - Error Handling Is One Thing

Functions should do one thing. Error handing is one thing. Thus, a function that handles
errors should do nothing else. This implies (as in the example above) that if the keyword
try exists in a function, it should be the very first word in the function and that there
should be nothing after the catch/finally blocks.

Comments - Good Comments

  • Legal Comments
// Copyright (C) 2003,2004,2005 by Object Mentor, Inc. All rights reserved.
// Released under the terms of the GNU General Public License version 2 or later.
  • Informative Comments: It is sometimes useful to provide basic information with a comment
// Returns an instance of the Responder being tested.
protected abstract Responder responderInstance();
  • 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.
}
  • Clarification
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
}
  • Warning of Consequences
// 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);
}
public static SimpleDateFormat makeStandardHttpDateFormat()
{
	//SimpleDateFormat is not thread safe,
	//so we need to create each instance independently.
	SimpleDateFormat df = new SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss z");
	df.setTimeZone(TimeZone.getTimeZone("GMT"));
	return df;
}
  • TODO Comments
//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;
}

Meaningful Names - Class Names

Clarity is king

Classes and objects should have noun or noun phrase names like Customer, WikiPage,
Account, and AddressParser. Avoid words like Manager, Processor, Data, or Info in the name
of a class. A class name should not be a verb.

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.