Skip to content

Commit

Permalink
Improve documentation for individual code smells
Browse files Browse the repository at this point in the history
- Use italics for code smell names
- Improve wording
- Link up smells to categories
- Unify configuration parameter tables
  • Loading branch information
mvz committed Apr 12, 2016
1 parent e500953 commit 8823290
Show file tree
Hide file tree
Showing 26 changed files with 179 additions and 143 deletions.
2 changes: 1 addition & 1 deletion docs/Attribute.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,4 @@ Reek does not raise warnings for read-only attributes.

## Configuration

`Attribute` supports only the [Basic Smell Options](Basic-Smell-Options.md).
_Attribute_ supports only the [Basic Smell Options](Basic-Smell-Options.md).
14 changes: 8 additions & 6 deletions docs/Boolean-Parameter.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@

## Introduction

`Boolean Parameter` is a special case of [Control Couple](Control-Couple.md), where a method parameter is defaulted
to true or false. A _Boolean Parameter_ effectively permits a method's caller
to decide which execution path to take. This is a case of bad cohesion. You're creating a dependency between methods that is not really necessary, thus increasing coupling.
_Boolean Parameter_ is a case of [Control Couple](Control-Couple.md), where a
method parameter is defaulted to true or false. A _Boolean Parameter_
effectively permits a method's caller to decide which execution path to take.
This is a case of bad cohesion. You're creating a dependency between methods
that is not really necessary, thus increasing coupling.

## Example

Expand Down Expand Up @@ -32,7 +34,7 @@ test.rb -- 3 warnings:
[2]:Dummy#hit_the_switch is controlled by argument switch (ControlParameter)
```

Note that both smells are reported, `Boolean Parameter` and `Control Parameter`.
Note that both smells are reported, _Boolean Parameter_ and _Control Parameter_.

## Getting rid of the smell

Expand All @@ -45,8 +47,8 @@ This is highly dependant on your exact architecture, but looking at the example

## Current support in Reek

Reek can only detect a Boolean parameter when it has a default initializer like in the example above.
Reek can only detect a _Boolean Parameter_ when it has a default initializer like in the example above.

## Configuration

`Boolean Parameter` supports the [Basic Smell Options](Basic-Smell-Options.md).
_Boolean Parameter_ supports the [Basic Smell Options](Basic-Smell-Options.md).
2 changes: 1 addition & 1 deletion docs/Class-Variable.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,4 @@ end

## Configuration

`Class Variable` supports the [Basic Smell Options](Basic-Smell-Options.md).
_Class Variable_ supports the [Basic Smell Options](Basic-Smell-Options.md).
22 changes: 13 additions & 9 deletions docs/Control-Couple.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,25 @@

## Introduction

Control coupling occurs when a method or block checks the value of a parameter in order to decide which execution path to take. The offending parameter is often called a `Control Couple`.
Control coupling occurs when a method or block checks the value of a parameter
in order to decide which execution path to take. The offending parameter is
often called a _Control Couple_.

Control Coupling is a kind of duplication, because the calling method already knows which path should be taken.

Control Coupling reduces the code's flexibility by creating a dependency between the caller and callee: any change to the possible values of the controlling parameter must be reflected on both sides of the call. A `Control Couple` also reveals a loss of simplicity: the called method probably has more than one responsibility, because it includes at least two different code paths.
Control Coupling reduces the code's flexibility by creating a dependency
between the caller and callee: any change to the possible values of the
controlling parameter must be reflected on both sides of the call. A _Control
Couple_ also reveals a loss of simplicity: the called method probably has more
than one responsibility, because it includes at least two different code paths.

You can find a good write-up regarding this problem [here](http://solnic.eu/2012/04/11/get-rid-of-that-code-smell-control-couple.html).

## Current Support in Reek

Reek warns about control coupling when:
Reek performs the following checks that fall in this category:

* [Control-Parameter](Control-Parameter.md) - a method parameter or block parameter is the tested value in a conditional statement (as in the example below); or
* [Boolean-Parameter](Boolean-Parameter.md) - a method parameter is defaulted to `true` or `false`.

## Configuration

Control Couple supports the [Basic Smell Options](Basic-Smell-Options.md).
* [Control-Parameter](Control-Parameter.md) - a method parameter or block
parameter is the tested value in a conditional statement
* [Boolean-Parameter](Boolean-Parameter.md) - a method parameter is defaulted
to `true` or `false`.
13 changes: 8 additions & 5 deletions docs/Control-Parameter.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@

## Introduction

`Control Parameter` is a special case of [Control Couple](Control-Couple.md)
_Control Parameter_ is a case of [Control Couple](Control-Couple.md)

## Example

A simple example would be the "quoted" parameter in the following method:
A simple example would be the `quoted` parameter in the following method:

```Ruby
def write(quoted)
Expand All @@ -18,12 +18,15 @@ def write(quoted)
end
```

Fixing those problems is out of the scope of this document but an easy solution could be to remove the "write" method alltogether and to move the calls to "write_quoted" / "write_unquoted" in the initial caller of "write".
Fixing those problems is out of the scope of this document but an easy solution
could be to remove the `write` method altogether and to move the calls to
`write_quoted` and `write_unquoted` to the caller of `write`.

## Current Support in Reek

Reek warns about control coupling when a method parameter or block parameter is the tested value in a conditional statement.
Reek warns about _Control Parameter_ when a method parameter or block parameter is
the tested value in a conditional statement.

## Configuration

Control Couple supports the [Basic Smell Options](Basic-Smell-Options.md).
_Control Parameter_ supports the [Basic Smell Options](Basic-Smell-Options.md).
16 changes: 9 additions & 7 deletions docs/Data-Clump.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

## Introduction

In general, a `Data Clump` occurs when the same two or three items frequently appear together in classes and parameter lists, or when a group of instance variable names start or end with similar substrings.
In general, a _Data Clump_ occurs when the same two or three items frequently
appear together in classes and parameter lists, or when a group of instance
variable names start or end with similar substrings.

The recurrence of the items often means there is duplicate code spread around to handle them. There may be an abstraction missing from the code, making the system harder to understand.

Expand All @@ -27,18 +29,18 @@ test.rb -- 1 warning:
A possible way to fix this problem (quoting from [Martin Fowler](http://martinfowler.com/bliki/DataClump.html)):
>> The first step is to replace data clumps with objects and use the objects whenever you see them. An immediate benefit is that you'll shrink some parameter lists. The interesting stuff happens as you begin to look for behavior to move into the new objects.
> The first step is to replace data clumps with objects and use the objects whenever you see them. An immediate benefit is that you'll shrink some parameter lists. The interesting stuff happens as you begin to look for behavior to move into the new objects.
## Current Support in Reek
Reek looks for a group of two or more parameters with the same names that are expected by three or more methods of a class.
## Configuration
Reek's Data Clump detector offers the [Basic Smell Options](Basic-Smell-Options.md), plus:
Reek's _Data Clump_ detector offers the [Basic Smell Options](Basic-Smell-Options.md), plus:
| Option | Value | Effect |
| ---------------|-------------|---------|
| max_copies | integer | The maximum number of methods that are permitted to take the same group of parameters. Defaults to 2 |
| min_clump_size | integer | The smallest number of parameters that can be reported as a clump. Defaults to 2 |
| Option | Value | Effect |
| -----------------|-------------|---------|
| `max_copies` | integer | The maximum number of methods that are permitted to take the same group of parameters. Defaults to 2. |
| `min_clump_size` | integer | The smallest number of parameters that can be reported as a clump. Defaults to 2. |
2 changes: 1 addition & 1 deletion docs/Duplicate-Method-Call.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ Reek's Duplicate Method Call detector checks for repeated identical method calls

## Configuration

Reek's Duplication detector currently offers the [Basic Smell Options](Basic-Smell-Options.md), plus:
Reek's Duplicate Method Call detector currently offers the [Basic Smell Options](Basic-Smell-Options.md), plus:

Option | Value | Effect
-------|-------|-------
Expand Down
7 changes: 4 additions & 3 deletions docs/Irresponsible-Module.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ Reek would emit the following warning:

```
test.rb -- 1 warning:
[1]:Dummy has no descriptive comment (IrresponsibleModule)
[1]:IrresponsibleModule: Dummy has no descriptive comment
```

Fixing this is simple - just an explaining comment:
Expand All @@ -32,8 +32,9 @@ end

## Current Support in Reek

`Irresponsible Module` currently checks classes, but not modules.
_Irresponsible Module_ checks classes and modules, including those
created through `Struct.new` and `Class.new` and directly assigned to a constant.

## Configuration

`Irresponsible Module` supports only the [Basic Smell Options](Basic-Smell-Options.md).
_Irresponsible Module_ supports only the [Basic Smell Options](Basic-Smell-Options.md).
19 changes: 7 additions & 12 deletions docs/Large-Class.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,14 @@

## Introduction

A `Large Class` is a class or module that has a large number of instance variables, methods or lines of code in any one piece of its specification. (That is, this smell relates to pieces of the class's specification, not to the size of the corresponding instance of `Class`.)
A _Large Class_ is a class or module that has a large number of instance
variables, methods or lines of code in any one piece of its specification.
(That is, this smell relates to pieces of the class's specification, not to the
size of the corresponding instance of `Class`.)

## Current Support in Reek

`Large Class` reports classes having more than a configurable number of methods or instance variables. The method count includes public, protected and private methods, and excludes methods inherited from superclasses or included modules.
Reek offers two checks in this category.

## Configuration

Reek's Large Class detector supports the [Basic Smell Options](Basic-Smell-Options.md), plus:

| Option | Value | Effect |
| ---------------|-------------|---------|
| max_methods | integer | The maximum number of methods allowed in a class before a warning is issued. Defaults to 25. |
| max_instance_variables | integer | The maximum number of instance variables allowed in a class before a warning is issued. Defaults to 9. |

The `Large Class` detector is enabled whenever Reek is asked to check an instance of `Class` or `Module`.
* [Too Many Instance Variables](Too-Many-Instance-Variables.md)
* [Too Many Methods](Too-Many-Methods.md)
13 changes: 7 additions & 6 deletions docs/Long-Parameter-List.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

## Introduction

A `Long Parameter List` occurs when a method has a lot of parameters.
A _Long Parameter List_ occurs when a method has a lot of parameters.

## Example

Expand All @@ -27,12 +27,13 @@ A common solution to this problem would be the introduction of parameter objects

## Current Support in Reek

`Long Parameter List` reports any method or block with more than 3 parameters.
_Long Parameter List_ reports any method or block with more than 3 parameters.

## Configuration

Reek's Long Parameter List detector supports the [Basic Smell Options](Basic-Smell-Options.md), plus:
Reek's _Long Parameter List_ detector supports the
[Basic Smell Options](Basic-Smell-Options.md), plus:

| Option | Value | Effect |
| ---------------|-------------|---------|
| max_params | integer | The maximum number of parameters allowed in a method or block before a warning is issued. Defaults to 3. |
| Option | Value | Effect |
| -------------|---------|---------|
| `max_params` | integer | The maximum number of parameters allowed in a method or block before a warning is issued. Defaults to 3. |
13 changes: 7 additions & 6 deletions docs/Long-Yield-List.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

## Introduction

A _Long Yield List_ occurs when a method yields a lot of arguments to the block it gets passed.
A _Long Yield List_ occurs when a method yields a lot of arguments to the block
it gets passed. It is a special case of [Long Parameter List](Long-Parameter-List.md).

## Example

Expand All @@ -25,12 +26,12 @@ A common solution to this problem would be the introduction of parameter objects

## Current Support in Reek

Currently Long Parameter List reports any method or block with more than 3 parameters.
Currently _Long Yield List_ reports any method or block with more than 3 parameters.

## Configuration

Reek's Long Parameter List detector supports the [Basic Smell Options](Basic-Smell-Options.md), plus:
Reek's _Long Yield List_ detector supports the [Basic Smell Options](Basic-Smell-Options.md), plus:

| Option | Value | Effect |
| ---------------|-------------|---------|
| max_params | integer | The maximum number of parameters allowed in a method or block before a warning is issued. Defaults to 3. |
| Option | Value | Effect |
| -------------|---------|---------|
| `max_params` | integer | The maximum number of parameters allowed in a method or block before a warning is issued. Defaults to 3. |
14 changes: 7 additions & 7 deletions docs/Nested-Iterators.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

## Introduction

A `Nested Iterator` occurs when a block contains another block.
A _Nested Iterator_ occurs when a block contains another block.

## Example

Expand Down Expand Up @@ -31,7 +31,7 @@ test.rb -- 1 warning:

## Current Support in Reek

Nested Iterators reports failing methods only once.
_Nested Iterators_ reports failing methods only once.
`Object#tap` is ignored by default and thus does not count as iterator.
Furthermore iterators without block arguments are not counted, e.g.:

Expand All @@ -51,9 +51,9 @@ would not smell of NestedIterators (given a maximum allowed nesting of 1) since

## Configuration

`Nested Iterators` offers the [Basic Smell Options](Basic-Smell-Options.md), plus:
_Nested Iterators_ offers the [Basic Smell Options](Basic-Smell-Options.md), plus:

| Option | Value | Effect |
| ---------------|-------------|---------|
| max_allowed_nesting | integer | The maximum depth of nested iterators. Defaults to 1 |
| ignore_iterators | Array | List of iterators to be excluded from the smell check. Includes only `tap` at the moment|
| Option | Value | Effect |
| ----------------------|---------|---------|
| `max_allowed_nesting` | integer | The maximum depth of nested iterators. Defaults to 1 |
| `ignore_iterators` | Array | List of iterators to be excluded from the smell check. Includes only `tap` at the moment|
12 changes: 8 additions & 4 deletions docs/Nil-Check.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,12 @@

## Introduction

A `NilCheck` is a type check. Failures of `NilCheck` violate the ["tell, don't ask"](http://robots.thoughtbot.com/tell-dont-ask) principle.
Additionally to that, type checks often mask bigger problems in your source code like not using OOP and / or polymorphism when you should.
A _Nil Check_ is a type check. Failures of _Nil Check_ violate the
["tell, don't ask"](http://robots.thoughtbot.com/tell-dont-ask) principle.
Additionally to that, type checks often mask bigger problems in your source
code like not using OOP and / or polymorphism when you should.

The _Nil Check_ code smell is a case of [Simulated Polymorphism](Simulated-Polymorphism.md).

## Example

Expand All @@ -28,12 +32,12 @@ test.rb -- 1 warning:

## Current Support in Reek

`NilCheck` reports use of
_Nil Check_ reports use of

* <code>.nil?</code> method
* <code>==</code> and <code>===</code> operators when checking vs. <code>nil</code>
* case statements that use syntax like <code>when nil</code>

## Configuration

`Nil Check` offers the [Basic Smell Options](Basic-Smell-Options.md).
_Nil Check_ offers the [Basic Smell Options](Basic-Smell-Options.md).
28 changes: 19 additions & 9 deletions docs/Prima-Donna-Method.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,24 @@

## Introduction

A candidate method for the `Prima Donna Method` smell are methods whose names end with an exclamation mark.
Candidate methods for the _Prima Donna Method_ smell are methods whose names
end with an exclamation mark.

An exclamation mark in method names means (the explanation below is taken from [here](http://dablog.rubypal.com/2007/8/15/bang-methods-or-danger-will-rubyist) ):
An exclamation mark in method names means (the explanation below is taken from
[here](http://dablog.rubypal.com/2007/8/15/bang-methods-or-danger-will-rubyist)):

>>
The ! in method names that end with ! means, “This method is dangerous”—or, more precisely, this method is the “dangerous” version of an otherwise equivalent method, with the same name minus the !. “Danger” is relative; the ! doesn’t mean anything at all unless the method name it’s in corresponds to a similar but bang-less method name.
So, for example, gsub! is the dangerous version of gsub. exit! is the dangerous version of exit. flatten! is the dangerous version of flatten. And so forth.
> The ! in method names that end with ! means, “This method is dangerous”—or,
> more precisely, this method is the “dangerous” version of an otherwise
> equivalent method, with the same name minus the !. “Danger” is relative; the
> ! doesn’t mean anything at all unless the method name it’s in corresponds to
> a similar but bang-less method name.
>
> So, for example, gsub! is the dangerous version of gsub. exit! is the
> dangerous version of exit. flatten! is the dangerous version of flatten. And
> so forth.
Such a method is called `Prima Donna Method` if and only if her non-bang version does not exist and this method is reported as a smell.
Such a method is called _Prima Donna Method_ if and only if her non-bang
version does not exist and this method is reported as a smell.

## Example

Expand All @@ -24,7 +33,7 @@ class C
end
```

Reek would report `bar!` as `prima donna method` smell but not `foo!`.
Reek would report the _Prima Donna Method_ smell for `bar!`, but not for `foo!`.

Reek reports this smell only in a class context, not in a module context in order to allow perfectly legit code like this:

Expand All @@ -46,8 +55,9 @@ class Daughter < Parent
end
```

In this example, Reek would not report the `prima donna method` smell for the method `foo` of the `Dangerous` module.
In this example, Reek would not report the _Prima Donna Method_ smell for the
method `foo` of the `Dangerous` module.

## Configuration

`Prima Donna Method` offers the [Basic Smell Options](Basic-Smell-Options.md).
_Prima Donna Method_ offers the [Basic Smell Options](Basic-Smell-Options.md).
7 changes: 5 additions & 2 deletions docs/Repeated-Conditional.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@

## Introduction

`Repeated Conditional` is a special case of [Simulated Polymorphism](Simulated-Polymorphism.md). Basically it means you are checking the same value throughout a single class and take decisions based on this.
_Repeated Conditional_ is a case of
[Simulated Polymorphism](Simulated-Polymorphism.md). Basically it means you are
checking the same value throughout a single class and take decisions based on
this.

## Example

Expand Down Expand Up @@ -37,7 +40,7 @@ If you get this warning then you are probably not using the right abstraction or

## Configuration

Reek's `Repeated Conditional` detector offers the [Basic Smell Options](Basic-Smell-Options.md), plus:
Reek's _Repeated Conditional_ detector offers the [Basic Smell Options](Basic-Smell-Options.md), plus:

| Option | Value | Effect |
| ---------------|-------------|---------|
Expand Down
Loading

0 comments on commit 8823290

Please sign in to comment.