clean-code-python/README.md at master · zedr/clean-code-python · GitHub

History

1611 lines (1124 loc) · 40.2 KB

Raw

100

101

102

103

104

105

106

107

108

109

110

111

112

113

114

115

116

117

118

119

120

121

122

123

124

125

126

127

128

129

130

131

132

133

134

135

136

137

138

139

140

141

142

143

144

145

146

147

148

149

150

151

152

153

154

155

156

157

158

159

160

161

162

163

164

165

166

167

168

169

170

171

172

173

174

175

176

177

178

179

180

181

182

183

184

185

186

187

188

189

190

191

192

193

194

195

196

197

198

199

200

201

202

203

204

205

206

207

208

209

210

211

212

213

214

215

216

217

218

219

220

221

222

223

224

225

226

227

228

229

230

231

232

233

234

235

236

237

238

239

240

241

242

243

244

245

246

247

248

249

250

251

252

253

254

255

256

257

258

259

260

261

262

263

264

265

266

267

268

269

270

271

272

273

274

275

276

277

278

279

280

281

282

283

284

285

286

287

288

289

290

291

292

293

294

295

296

297

298

299

300

301

302

303

304

305

306

307

308

309

310

311

312

313

314

315

316

317

318

319

320

321

322

323

324

325

326

327

328

329

330

331

332

333

334

335

336

337

338

339

340

341

342

343

344

345

346

347

348

349

350

351

352

353

354

355

356

357

358

359

360

361

362

363

364

365

366

367

368

369

370

371

372

373

374

375

376

377

378

379

380

381

382

383

384

385

386

387

388

389

390

391

392

393

394

395

396

397

398

399

400

401

402

403

404

405

406

407

408

409

410

411

412

413

414

415

416

417

418

419

420

421

422

423

424

425

426

427

428

429

430

431

432

433

434

435

436

437

438

439

440

441

442

443

444

445

446

447

448

449

450

451

452

453

454

455

456

457

458

459

460

461

462

463

464

465

466

467

468

469

470

471

472

473

474

475

476

477

478

479

480

481

482

483

484

485

486

487

488

489

490

491

492

493

494

495

496

497

498

499

500

501

502

503

504

505

506

507

508

509

510

511

512

513

514

515

516

517

518

519

520

521

522

523

524

525

526

527

528

529

530

531

532

533

534

535

536

537

538

539

540

541

542

543

544

545

546

547

548

549

550

551

552

553

554

555

556

557

558

559

560

561

562

563

564

565

566

567

568

569

570

571

572

573

574

575

576

577

578

579

580

581

582

583

584

585

586

587

588

589

590

591

592

593

594

595

596

597

598

599

600

601

602

603

604

605

606

607

608

609

610

611

612

613

614

615

616

617

618

619

620

621

622

623

624

625

626

627

628

629

630

631

632

633

634

635

636

637

638

639

640

641

642

643

644

645

646

647

648

649

650

651

652

653

654

655

656

657

658

659

660

661

662

663

664

665

666

667

668

669

670

671

672

673

674

675

676

677

678

679

680

681

682

683

684

685

686

687

688

689

690

691

692

693

694

695

696

697

698

699

700

701

702

703

704

705

706

707

708

709

710

711

712

713

714

715

716

717

718

719

720

721

722

723

724

725

726

727

728

729

730

731

732

733

734

735

736

737

738

739

740

741

742

743

744

745

746

747

748

749

750

751

752

753

754

755

756

757

758

759

760

761

762

763

764

765

766

767

768

769

770

771

772

773

774

775

776

777

778

779

780

781

782

783

784

785

786

787

788

789

790

791

792

793

794

795

796

797

798

799

800

801

802

803

804

805

806

807

808

809

810

811

812

813

814

815

816

817

818

819

820

821

822

823

824

825

826

827

828

829

830

831

832

833

834

835

836

837

838

839

840

841

842

843

844

845

846

847

848

849

850

851

852

853

854

855

856

857

858

859

860

861

862

863

864

865

866

867

868

869

870

871

872

873

874

875

876

877

878

879

880

881

882

883

884

885

886

887

888

889

890

891

892

893

894

895

896

897

898

899

900

901

902

903

904

905

906

907

908

909

910

911

912

913

914

915

916

917

918

919

920

921

922

923

924

925

926

927

928

929

930

931

932

933

934

935

936

937

938

939

940

941

942

943

944

945

946

947

948

949

950

951

952

953

954

955

956

957

958

959

960

961

962

963

964

965

966

967

968

969

970

971

972

973

974

975

976

977

978

979

980

981

982

983

984

985

986

987

988

989

990

991

992

993

994

995

996

997

998

999

1000

# clean-code-python

[![Build Status](https://travis-ci.com/zedr/clean-code-python.svg?branch=master)](https://travis-ci.com/zedr/clean-code-python)

[![](https://img.shields.io/badge/python-3.8+-blue.svg)](https://www.python.org/download/releases/3.8.3/)

## Table of Contents

1. [Introduction](#introduction)

2. [Variables](#variables)

3. [Functions](#functions)

5. [Classes](#classes)

1. [S: Single Responsibility Principle (SRP)](#single-responsibility-principle-srp)

2. [O: Open/Closed Principle (OCP)](#openclosed-principle-ocp)

3. [L: Liskov Substitution Principle (LSP)](#liskov-substitution-principle-lsp)

4. [I: Interface Segregation Principle (ISP)](#interface-segregation-principle-isp)

5. [D: Dependency Inversion Principle (DIP)](#dependency-inversion-principle-dip)

6. [Don't repeat yourself (DRY)](#dont-repeat-yourself-dry)

7. [Translations](#translations)

## Introduction

Software engineering principles, from Robert C. Martin's book

[*Clean

Code*](https://www.amazon.com/Clean-Code-Handbook-Software-Craftsmanship/dp/0132350882)

, adapted for Python. This is not a style guide. It's a guide to producing

readable, reusable, and refactorable software in Python.

Not every principle herein has to be strictly followed, and even fewer will be

universally agreed upon. These are guidelines and nothing more, but they are

ones codified over many years of collective experience by the authors of *Clean

Code*.

Adapted

from [clean-code-javascript](https://github.com/ryanmcdermott/clean-code-javascript)

Targets Python3.7+

## **Variables**

### Use meaningful and pronounceable variable names

**Bad:**

```python

import datetime

ymdstr = datetime.date.today().strftime("%y-%m-%d")

```

Additionally, there's no need to add the type of the variable (str) to its

name.

**Good**:

```python

import datetime

current_date: str = datetime.date.today().strftime("%y-%m-%d")

```

**[⬆ back to top](#table-of-contents)**

### Use the same vocabulary for the same type of variable

**Bad:**

Here we use three different names for the same underlying entity:

```python

def get_user_info(): pass

def get_client_data(): pass

def get_customer_record(): pass

```

**Good**:

If the entity is the same, you should be consistent in referring to it in your

functions:

```python

def get_user_info(): pass

def get_user_data(): pass

def get_user_record(): pass

```

**Even better**

Python is (also) an object oriented programming language. If it makes sense,

package the functions together with the concrete implementation of the entity

in your code, as instance attributes, property methods, or methods:

```python

from typing import Union, Dict

class Record:

pass

class User:

info: str

@property

def data(self) -> Dict[str, str]:

return {}

def get_record(self) -> Union[Record, None]:

return Record()

```

**[⬆ back to top](#table-of-contents)**

### Use searchable names

We will read more code than we will ever write. It's important that the code we

do write is readable and searchable. By *not* naming variables that end up

being meaningful for understanding our program, we hurt our readers. Make your

names searchable.

**Bad:**

```python

import time

# What is the number 86400 for again?

time.sleep(86400)

```

**Good**:

```python

import time

# Declare them in the global namespace for the module.

SECONDS_IN_A_DAY = 60 * 60 * 24

time.sleep(SECONDS_IN_A_DAY)

```

**[⬆ back to top](#table-of-contents)**

### Use explanatory variables

**Bad:**

```python

import re

address = "One Infinite Loop, Cupertino 95014"

city_zip_code_regex = r"^[^,\\]+[,\\\s]+(.+?)\s*(\d{5})?$"

matches = re.match(city_zip_code_regex, address)

if matches:

print(f"{matches[1]}: {matches[2]}")

```

**Not bad**:

It's better, but we are still heavily dependent on regex.

```python

import re

address = "One Infinite Loop, Cupertino 95014"

city_zip_code_regex = r"^[^,\\]+[,\\\s]+(.+?)\s*(\d{5})?$"

matches = re.match(city_zip_code_regex, address)

if matches:

city, zip_code = matches.groups()

print(f"{city}: {zip_code}")

```

**Good**:

Decrease dependence on regex by naming subpatterns.

```python

import re

address = "One Infinite Loop, Cupertino 95014"

city_zip_code_regex = r"^[^,\\]+[,\\\s]+(?P<city>.+?)\s*(?P<zip_code>\d{5})?$"

matches = re.match(city_zip_code_regex, address)

if matches:

print(f"{matches['city']}, {matches['zip_code']}")

```

**[⬆ back to top](#table-of-contents)**

### Avoid Mental Mapping

Don’t force the reader of your code to translate what the variable means.

Explicit is better than implicit.

**Bad:**

```python

seq = ("Austin", "New York", "San Francisco")

for item in seq:

# do_stuff()

# do_some_other_stuff()

# Wait, what's `item` again?

print(item)

```

**Good**:

```python

locations = ("Austin", "New York", "San Francisco")

for location in locations:

# do_stuff()

# do_some_other_stuff()

# ...

print(location)

```

**[⬆ back to top](#table-of-contents)**

### Don't add unneeded context

If your class/object name tells you something, don't repeat that in your

variable name.

**Bad:**

```python

class Car:

car_make: str

car_model: str

car_color: str

```

**Good**:

```python

class Car:

make: str

model: str

color: str

```

**[⬆ back to top](#table-of-contents)**

### Use default arguments instead of short circuiting or conditionals

**Tricky**

Why write:

```python

import hashlib

def create_micro_brewery(name):

name = "Hipster Brew Co." if name is None else name

slug = hashlib.sha1(name.encode()).hexdigest()

# etc.

```

... when you can specify a default argument instead? This also makes it clear

that you are expecting a string as the argument.

**Good**:

```python

import hashlib

def create_micro_brewery(name: str = "Hipster Brew Co."):

slug = hashlib.sha1(name.encode()).hexdigest()

# etc.

```

**[⬆ back to top](#table-of-contents)**

## **Functions**

### Functions should do one thing

This is by far the most important rule in software engineering. When functions

do more than one thing, they are harder to compose, test, and reason about.

When you can isolate a function to just one action, they can be refactored

easily and your code will read much cleaner. If you take nothing else away from

this guide other than this, you'll be ahead of many developers.

**Bad:**

```python

from typing import List

class Client:

active: bool

def email(client: Client) -> None:

pass

def email_clients(clients: List[Client]) -> None:

"""Filter active clients and send them an email.

"""

for client in clients:

if client.active:

email(client)

```

**Good**:

```python

from typing import List

class Client:

active: bool

def email(client: Client) -> None:

pass

def get_active_clients(clients: List[Client]) -> List[Client]:

"""Filter active clients.

"""

return [client for client in clients if client.active]

def email_clients(clients: List[Client]) -> None:

"""Send an email to a given list of clients.

"""

for client in get_active_clients(clients):

email(client)

```

Do you see an opportunity for using generators now?

**Even better**

```python

from typing import Generator, Iterator

class Client:

active: bool

def email(client: Client):

pass

def active_clients(clients: Iterator[Client]) -> Generator[Client, None, None]:

"""Only active clients"""

return (client for client in clients if client.active)

def email_client(clients: Iterator[Client]) -> None:

"""Send an email to a given list of clients.

"""

for client in active_clients(clients):

email(client)

```

**[⬆ back to top](#table-of-contents)**

### Function arguments (2 or fewer ideally)

A large amount of parameters is usually the sign that a function is

doing too much (has more than one responsibility). Try to decompose it

into smaller functions having a reduced set of parameters, ideally less than

three.

If the function has a single responsibility, consider if you can bundle

some or all parameters into a specialized object that will be passed as an

argument to the function. These parameters might be attributes of a single

entity that you can represent with a dedicated data structure. You may also

be able to reuse this entity elsewhere in your program. The reason why this is

a better arrangement is than having multiple parameters is that we may be able

to move some computations, done with those parameters inside the

function, into methods belonging to the new object, therefore reducing the

complexity of the function.

**Bad:**

```python

def create_menu(title, body, button_text, cancellable):

pass

```

**Java-esque**:

```python

class Menu:

def __init__(self, config: dict):

self.title = config["title"]

self.body = config["body"]

# ...

menu = Menu(

{

"title": "My Menu",

"body": "Something about my menu",

"button_text": "OK",

"cancellable": False

}

)

```

**Also good**

```python

class MenuConfig:

"""A configuration for the Menu.

Attributes:

title: The title of the Menu.

body: The body of the Menu.

button_text: The text for the button label.

cancellable: Can it be cancelled?

"""

title: str

body: str

button_text: str

cancellable: bool = False

def create_menu(config: MenuConfig) -> None:

title = config.title

body = config.body

# ...

config = MenuConfig()

config.title = "My delicious menu"

config.body = "A description of the various items on the menu"

config.button_text = "Order now!"

# The instance attribute overrides the default class attribute.

config.cancellable = True

create_menu(config)

```

**Fancy**

```python

from typing import NamedTuple

class MenuConfig(NamedTuple):

"""A configuration for the Menu.

Attributes:

title: The title of the Menu.

body: The body of the Menu.

button_text: The text for the button label.

cancellable: Can it be cancelled?

"""

title: str

body: str

button_text: str

cancellable: bool = False

def create_menu(config: MenuConfig):

title, body, button_text, cancellable = config

# ...

create_menu(

MenuConfig(

title="My delicious menu",

body="A description of the various items on the menu",

button_text="Order now!"

)

```

**Even fancier**

```python

from dataclasses import astuple, dataclass

@dataclass

class MenuConfig:

"""A configuration for the Menu.

Attributes:

title: The title of the Menu.

body: The body of the Menu.

button_text: The text for the button label.

cancellable: Can it be cancelled?

"""

title: str

body: str

button_text: str

cancellable: bool = False

def create_menu(config: MenuConfig):

title, body, button_text, cancellable = astuple(config)

# ...

create_menu(

MenuConfig(

title="My delicious menu",

body="A description of the various items on the menu",

button_text="Order now!"

)

```

**Even fancier, Python3.8+ only**

```python

from typing import TypedDict

class MenuConfig(TypedDict):

"""A configuration for the Menu.

Attributes:

title: The title of the Menu.

body: The body of the Menu.

button_text: The text for the button label.

cancellable: Can it be cancelled?

"""

title: str

body: str

button_text: str

cancellable: bool

def create_menu(config: MenuConfig):

title = config["title"]

# ...

create_menu(

# You need to supply all the parameters

MenuConfig(

title="My delicious menu",

body="A description of the various items on the menu",

button_text="Order now!",

cancellable=True

)

```

**[⬆ back to top](#table-of-contents)**

### Function names should say what they do

**Bad:**

```python

class Email:

def handle(self) -> None:

pass

message = Email()

# What is this supposed to do again?

message.handle()

```

**Good:**

```python

class Email:

def send(self) -> None:

"""Send this message"""

message = Email()

message.send()

```

**[⬆ back to top](#table-of-contents)**

### Functions should only be one level of abstraction

When you have more than one level of abstraction, your function is usually

doing too much. Splitting up functions leads to reusability and easier testing.

**Bad:**

```python

# type: ignore

def parse_better_js_alternative(code: str) -> None:

regexes = [

# ...

]

statements = code.split('\n')

tokens = []

for regex in regexes:

for statement in statements:

pass

ast = []

for token in tokens:

pass

for node in ast:

pass

```

**Good:**

```python

from typing import Tuple, List, Dict

REGEXES: Tuple = (

# ...

)

def parse_better_js_alternative(code: str) -> None:

tokens: List = tokenize(code)

syntax_tree: List = parse(tokens)

for node in syntax_tree:

pass

def tokenize(code: str) -> List:

statements = code.split()

tokens: List[Dict] = []

for regex in REGEXES:

for statement in statements:

pass

return tokens

def parse(tokens: List) -> List:

syntax_tree: List[Dict] = []

for token in tokens:

pass

return syntax_tree

```

**[⬆ back to top](#table-of-contents)**

### Don't use flags as function parameters

Flags tell your user that this function does more than one thing. Functions

should do one thing. Split your functions if they are following different code

paths based on a boolean.

**Bad:**

```python

from tempfile import gettempdir

from pathlib import Path

def create_file(name: str, temp: bool) -> None:

if temp:

(Path(gettempdir()) / name).touch()

else:

Path(name).touch()

```

**Good:**

```python

from tempfile import gettempdir

from pathlib import Path

def create_file(name: str) -> None:

Path(name).touch()

def create_temp_file(name: str) -> None:

(Path(gettempdir()) / name).touch()

```

**[⬆ back to top](#table-of-contents)**

### Avoid side effects

A function produces a side effect if it does anything other than take a value

in and return another value or values. For example, a side effect could be

writing to a file, modifying some global variable, or accidentally wiring all

your money to a stranger.

Now, you do need to have side effects in a program on occasion - for example,

like in the previous example, you might need to write to a file. In these

cases, you should centralize and indicate where you are incorporating side

effects. Don't have several functions and classes that write to a particular

file - rather, have one

(and only one) service that does it.

The main point is to avoid common pitfalls like sharing state between objects

without any structure, using mutable data types that can be written to by

anything, or using an instance of a class, and not centralizing where your side

effects occur. If you can do this, you will be happier than the vast majority

of other programmers.

**Bad:**

```python

# type: ignore

# This is a module-level name.

# It's good practice to define these as immutable values, such as a string.

# However...

fullname = "Ryan McDermott"

def split_into_first_and_last_name() -> None:

# The use of the global keyword here is changing the meaning of the

# the following line. This function is now mutating the module-level

# state and introducing a side-effect!

global fullname

fullname = fullname.split()

split_into_first_and_last_name()

# MyPy will spot the problem, complaining about 'Incompatible types in

# assignment: (expression has type "List[str]", variable has type "str")'

print(fullname) # ["Ryan", "McDermott"]

# OK. It worked the first time, but what will happen if we call the

# function again?

```

**Good:**

```python

from typing import List, AnyStr

def split_into_first_and_last_name(name: AnyStr) -> List[AnyStr]:

return name.split()

fullname = "Ryan McDermott"

name, surname = split_into_first_and_last_name(fullname)

print(name, surname) # => Ryan McDermott

```

**Also good**

```python

from dataclasses import dataclass

@dataclass

class Person:

name: str

@property

def name_as_first_and_last(self) -> list:

return self.name.split()

# The reason why we create instances of classes is to manage state!

person = Person("Ryan McDermott")

print(person.name) # => "Ryan McDermott"

print(person.name_as_first_and_last) # => ["Ryan", "McDermott"]

```

**[⬆ back to top](#table-of-contents)**

## **Classes**

### **Single Responsibility Principle (SRP)**

Robert C. Martin writes:

> A class should have only one reason to change.

"Reasons to change" are, in essence, the responsibilities managed by a class or

function.

In the following example, we create an HTML element that represents a comment

with the version of the document:

**Bad**

```python

from importlib import metadata

class VersionCommentElement:

"""An element that renders an HTML comment with the program's version number

"""

def get_version(self) -> str:

"""Get the package version"""

return metadata.version("pip")

def render(self) -> None:

print(f'')

VersionCommentElement().render()

```

This class has two responsibilities:

- Retrieve the version number of the Python package

- Render itself as an HTML element

Any change to one or the other carries the risk of impacting the other.

We can rewrite the class and decouple these responsibilities:

**Good**

```python

from importlib import metadata

def get_version(pkg_name: str) -> str:

"""Retrieve the version of a given package"""

return metadata.version(pkg_name)

class VersionCommentElement:

"""An element that renders an HTML comment with the program's version number

"""

def __init__(self, version: str):

self.version = version

def render(self) -> None:

print(f'')

VersionCommentElement(get_version("pip")).render()

```

The result is that the class only needs to take care of rendering itself. It

receives the version text during instantiation and this text is generated by

calling a separate function, `get_version()`. Changing the class has no impact

on the other, and vice-versa, as long as the contract between them does not

change, i.e. the function provides a string and the class `__init__` method

accepts a string.

As an added bonus, the `get_version()` is now reusable elsewhere.

### **Open/Closed Principle (OCP)**

> “Incorporate new features by extending the system, not by making

> modifications (to it)”,

> Uncle Bob.

Objects should be open for extension, but closed to modification. It should be

possible to augment the functionality provided by an object (for example, a

class)

without changing its internal contracts. An object can enable this when it is

designed to be extended cleanly.

In the following example, we try to implement a simple web framework that

handles HTTP requests and returns responses. The `View` class has a single

method `.get()` that will be called when the HTTP server will receive a GET

request from a client.

`View` is intentionally simple and returns `text/plain` responses. We would

also like to return HTML responses based on a template file, so we subclass it

using the `TemplateView` class.

**Bad**

```python

from dataclasses import dataclass

@dataclass

class Response:

"""An HTTP response"""

status: int

content_type: str

body: str

class View:

"""A simple view that returns plain text responses"""

def get(self, request) -> Response:

"""Handle a GET request and return a message in the response"""

return Response(

status=200,

content_type='text/plain',

body="Welcome to my web site"

)

class TemplateView(View):

"""A view that returns HTML responses based on a template file."""

def get(self, request) -> Response:

"""Handle a GET request and return an HTML document in the response"""

with open("index.html") as fd:

return Response(

status=200,

content_type='text/html',

body=fd.read()

)

```

The `TemplateView` class has modified the internal behaviour of its parent

class in order to enable the more advanced functionality. In doing so, it now

relies on the `View` to not change the implementation of the `.get()`

method, which now needs to be frozen in time. We cannot introduce, for example,

some additional checks in all our `View`-derived classes because the behaviour

is overridden in at least one subtype and we will need to update it.

Let's redesign our classes to fix this problem and let the `View` class be

extended (not modified) cleanly:

**Good**

```python

from dataclasses import dataclass

@dataclass

class Response:

"""An HTTP response"""

status: int

content_type: str

body: str

class View:

"""A simple view that returns plain text responses"""

content_type = "text/plain"

def render_body(self) -> str:

"""Render the message body of the response"""

return "Welcome to my web site"

def get(self, request) -> Response:

"""Handle a GET request and return a message in the response"""

return Response(

status=200,

content_type=self.content_type,

body=self.render_body()

)

class TemplateView(View):

"""A view that returns HTML responses based on a template file."""

content_type = "text/html"

template_file = "index.html"

def render_body(self) -> str:

"""Render the message body as HTML"""

with open(self.template_file) as fd:

return fd.read()

```

Note that we did need to override the `render_body()` in order to change the

source of the body, but this method has a single, well defined responsibility

that **invites subtypes to override it**. It is designed to be extended by its

subtypes.

Another good way to use the strengths of both object inheritance and object

composition is to

use [Mixins](https://docs.djangoproject.com/en/4.1/topics/class-based-views/mixins/)

Mixins are bare-bones classes that are meant to be used exclusively with other

related classes. They are "mixed-in" with the target class using multiple

inheritance, in order to change the target's behaviour.

A few rules:

- Mixins should always inherit from `object`

- Mixins always come before the target class,

e.g. `class Foo(MixinA, MixinB, TargetClass): ...`

**Also good**

```python

View remainder of file in raw view

clean-code-python/README.md at master · zedr/clean-code-python · GitHub

Expand file tree

Breadcrumbs

README.md

Latest commit

History

Breadcrumbs

README.md

File metadata and controls