Approaching a Legacy System

class: center, middle
# Approaching a Legacy System

by Emmanuel Delgado

???
My name is Emmanuel Delgado, I am a Software Developer and I work for modcloth.com.
---

# Agenda

1. Definition and mindset
2. Just look at the files
3. Become a Git historian
4. Look for documentation
5. Understand your boundaries
6. The daily boy scout
7. Static analysis tools
8. Understand your tests
9. Extract features
10. Adopt a few core ideas

???
This is how the general talk is gonna go, it will take around 20 mins, with a mix of ideas, stories,
and tools/code.
---

# 1. Definition and mindset

## Wikipedia definition
> In computing, a legacy system is an **old** method, **technology**, **computer system**, or **application program**, "of, relating to, or being a previous or **outdated computer system**." Often a pejorative term, referencing a system as "legacy" means that it paved the way for the standards that would follow it. This can also imply that the system is out of date or **in need of replacement.**

---

.grayed-out[
  # 1. Definition and mindset
]

## Get ready for an archeology trip
- Start writing down notes of everything you are learning
- It does not have to be a book, keywords may be enough
- Mind maps are great

.center.mindmap[![](dev-plan.jpg)]
---

# 2. Just look at the files
- Use **ls** to look for clues on what the domain knowledge looks like:

```bash
$ ls
Capfile              README.md            config.ru            public
Gemfile              Rakefile             db                   script
Gemfile.lock         Vagrantfile          doc                  spec
Guardfile            app                  features             spec.acceptance
Procfile             certs                lib                  vendor
Procfile.development config               log
```

```bash
$ ls app
assets      helpers     middleware  presenters  stylesheets
controllers jobs        models      services    views
```
---

.grayed-out[
  # 2. Just look at the files
]
- Use **tree** to look deeper:

```bash
$ tree app lib
lib
├── activate_toggle_switch.rb
├── active_merchant
│   └── billing
│       ├── braintree_mod_gateway.rb
│       └── gateways
│           └── modcloth_bogus.rb
├── active_record_selective_error_presenter.rb
├── active_shipping
│   ├── active_shipping
│   │   ├── lib
│   │   │   ├── country.rb
│   │   │   ├── error.rb
...
└── wsdl
    ├── ShipServiceDevelopment_v9.wsdl

504 directories, 3052 files
```
---

.grayed-out[
  # 2. Just look at the files
]
- Use **ag**, **wc**. It looks like rails and ruby so, how much ruby?:
  ```bash
  $ ag -l 'class|module' --ruby | wc
    3227    3227  157092
  ```

- How much ruby in *lib* and *app*?:
  ```bash
  $ ag -l 'class|module' --ruby  app lib |wc
    1836    1836   79395
  ```

- How many files types are there [thank you stackoverflow](https://stackoverflow.com/questions/1842254/how-can-i-find-all-of-the-distinct-file-extensions-in-a-folder-hierarchy)?
  ```bash
  $ find . -type f \
    | sed -e 's/.*\.//' \
    | sed -e 's/.*\///' \
    | sort | uniq -c | sort -rn| wc
       217     434    3881
  ```
---

.grayed-out[
  # 2. Just look at the files
]
- Use **find**, **sort**, **sed**, **uniq**, **head** to get the top counts per file type:
  ```bash
  $ find . -type f \
    | sed -e 's/.*\.//' \
    | sed -e 's/.*\///' \
    | sort | uniq -c | sort -rn
    | head -n10
  3579 rb
  1634 png
   829 js
   775 erb
   586 gif
   547 jpg
   511 haml
   372 scss
   248 gem
   124 css
    81 feature
    77 rake
    58 csv
    52 yml
    41 html
    39 htm
    38 xml
    17 svg
  ```
---

# 3. Become a Git historian
- Use **git-extras** to get a summary of the project
  ```bash
  $ brew upgrade git-extras
  Error: git-extras 4.3.0 already installed
  $ git summary
  git summary |more

project  : e-comm
   repo age : 9 years
   active   : 2420 days
   commits  : 42828
   files    : 9942
   authors  :
    1806  Person 1  4.2%
    1566  Person 2  3.7%
    1431  Person 3  3.3%
    1218  Person 4  2.8%
    1072  Person 5  2.5%
     962  Person 6  2.2%
     878  Person 7  2.1%
     729  Person 8  1.7%
     717  Person 9  1.7%
     711  Person 10 1.7%
     674  Person 11 1.6%
  ```
---

.grayed-out[
  # 3. Become a Git historian
]
* Aggregate results with **ag**, **tr**, **sort**, **uniq** and **wc**
  ```bash
  git summary \
    | ag -o '[a-zA-Z\s]+' \
    | tr '[:upper:]' '[:lower:]' \
    | ag -o '\w+(\s\w+)?' \
    | sort | uniq| wc
       325     522    3715
  ```
- You can also go to github in case it is stored in github
- Or use the cool https://github.com/IonicaBizau/git-stats
---

.grayed-out[
  # 3. Become a Git historian
]
- Understand change on single objects with **git log**:

```bash
$ git log --patch --stat Gemfile
commit ae961aee1ec7502f14f94e938b93c2b031547e39
Author: x <x@place>
Date:   Fri Jan 20 18:58:54 2017 -0600

WIP
---
Gemfile | 1 +
1 file changed, 1 insertion(+)

diff --git a/Gemfile b/Gemfile
index 507ae3f1ee..3fa2eec31a 100644
--- a/Gemfile
+++ b/Gemfile
@@ -28,6 +28,7 @@ gem 'rpm_contrib'
gem 'newrelic_rpm'
gem 'mongrel', '>=1.0.2.pre2'
gem 'mysql2', '0.3.17' # above 0.3.16 because of https://github.com/brianmario/mysql2/issues/711#issuecomment-163438185
+gem 'net-sftp'
gem 'nokogiri'
gem 'oauth-plugin', '~> 0.4.1' # Need to specify the version or bundler will install an old one.
gem 'packet', '0.1.15'

commit e7237329cd0c0629b660697bcdf69e9328258e54
```
---

# 4. Look for documentation
As we look for documentation keep in mind:

- Is there any documentation at all?
- Is there good commentary on the code?
- Does the names of things make sense?
- Are the system use cases are documented?
- Is there someone in the team that understand the system?
---

.grayed-out[
  # 4. Look for documentation
]
- Seems that we have a *doc* folder:
  ```bash
  $ git whatchanged doc
  commit d2968ae8573095ed32676f11dbc3a97d77d0549d
  Author: Person X
  Date:   Wed Jun 11 12:58:30 2014 -0400

[SB] Delete random file?

:100644 000000 ab1b12981e... 0000000000... D    doc/obsolete/BCREC.csv
  ```

- The last document modified on 2014, so it's time to start your documentation.
---

.grayed-out[
  # 4. Look for documentation
]
## Here are a few things to keep in mind:

- Document everything you learn.
- Keep documentation close to the code, simple is better.
- You must understand the design/architecture.
- Once you understand something refine it's documentation.

???
- One common problem for systems that have been alive for a few years and sometimes even younger systems is that every new developer.
- New designs on top of the existing ones without refactoring, then they get bored or leave and now we have new designs on top of the older ones.
---

# 5. Understand your boundaries
One way to help you understand the architecture is to understand the boundaries. Let's
start with your *Gemfile*:
```ruby
source 'https://rubygems.org'

ruby '2.0.0'

gem 'rails', '~>3.2.17'

gem 'active_presenter'
gem 'activemerchant', '~>1.42'
gem 'ancestry', '>=1.2.4'
gem 'acts-as-taggable-on', '~> 3.4.2'
gem 'ambethia-smtp-tls', '1.1.2', :require => 'smtp-tls'
gem 'american_date'
gem 'aws-sdk'
gem 'bitly'
gem 'braintree', '2.32.1'
gem 'yajl-ruby'
gem 'chronic'
gem 'curb'
gem 'haml-rails'
gem 'haml-contrib' # we need this for the Textile filter, that was moved out of core HAML
gem 'simple_form' #, github: 'plataformatec/simple_form', branch: 'v2.2' # admin dependency
...
```
---

.grayed-out[
  # 5. Understand your boundaries
]

- A naive filter would give me a better idea:
  ```bash
  $ ag -o "\'[\w-]+\'" Gemfile \
    | tr "'" " " \
    | sort \
    | wc
       188     188    2675
  ```

.left-column[
    ```bash
    $ ag -o "\'[\w-]+\'" Gemfile \
      | tr "'" " " \
      | sort
     activemerchant
     activerecord-import
     activerecord-nulldb-adapter
     acts-as-taggable-on
     acts_as_list
     addressable
     airbrake
     ambethia-smtp-tls
     american_date
     ancestry
     ap
     awesome_nested_set
     awesome_print
    ```
   ]

.right-colum[
    ```bash
     capistrano
     capistrano-ext
     capybara
     capybara-screenshot
     capybara-webkit
     chronic
     codeclimate-test-reporter
     colorize
     compass
     compass-rails
     connection_pool
     cookie_slasher
     cucumber-rails
     curb
     ...
     typhoeus
     validates_timeliness
    ```
  ]
---

.grayed-out[
  # 5. Understand your boundaries
]

- Remember your notes
- Draw how the boundaries look like
- You don't need to follow a specific methodology, just draw

.center.diagraming[![](diagraming.jpg)]

---

.grayed-out[
  # 5. Understand your boundaries
]

- Try [rubrowser](http://www.blazeboy.me/rubrowser/)

.center.rubrowser[![](sg-rubrowser.gif)]

???
- Pending what do you understand from rubrowser?
---

# 6. The daily boy scout
I heard it from [Uncle Bob](http://programmer.97things.oreilly.com/wiki/index.php/The_Boy_Scout_Rule):

> Always leave the campground cleaner than you found it.

- Excuses are many, get over them
- Now let's figure out where to start
---

# 7. Static analysis tools
There is [metric_fu](https://github.com/metricfu/metric_fu), but let's take a look to only a few of its tools, plus my favorite [rubocop](https://github.com/bbatsov/rubocop) and as a bonus [unused](https://github.com/joshuaclayton/unused).

- churn
- reek
- code_metrics
- [rubocop](https://github.com/bbatsov/rubocop)
- [unused](https://github.com/joshuaclayton/unused)
---

.grayed-out[
  # 7. Static analysis tools - **churn**
]

> High method and class churn has been shown to have increased bug and error rates ...

```bash
$ churn

Files
+-------------------------------------------------------+---------------+
| file_path                                             | times_changed |
+-------------------------------------------------------+---------------+
| ...o_inventory_snapshot_job.rb                        | 10            |
| ...duct_manager/sync_for_new_or_updated_products.rb   | 7             |
| ...oduct.rb                                           | 6             |
| ...s.rake                                             | 6             |

.grayed-out[
  # 7. Static analysis tools - **reek**
]

> Reek is a tool that examines Ruby classes, modules and methods and reports any code smells it finds.

```bash
$ reek --format json app/models/wishlist.rb
```
```json
  {
    "context": "Wishlist#last_added_product",
    "lines": [
      63,
      63
    ],
    "message": "calls 'items.last_added' 2 times",
    "smell_type": "DuplicateMethodCall",
    "source": "app/models/wishlist.rb",
    "name": "items.last_added",
    "count": 2,
    "wiki_link": "https://github.com/troessner/reek/blob/master/docs/Duplicate-Method-Call.md"
  },
  {
    "context": "Wishlist#move_to_cart",
    "lines": [
      26,
      28,
      32
    ],
    "message": "refers to 'cart' more than self (maybe move it to another class?)",
    "smell_type": "FeatureEnvy",
    "source": "app/models/wishlist.rb",
    "name": "cart",
    "wiki_link": "https://github.com/troessner/reek/blob/master/docs/Feature-Envy.md"
  },
```
---

.grayed-out[
  # 7. Static analysis tools - **reek**
]

You could quickly look for what code smells there are in a bunch of files, just to see how
stinky the code base is:

```bash
  $ reek --format json app/models | jq '.[] .smell_type'| sort | uniq
  "Attribute"
  "BooleanParameter"
  "ClassVariable"
  "ControlParameter"
  "DataClump"
  "DuplicateMethodCall"
  "FeatureEnvy"
  "InstanceVariableAssumption"
  "IrresponsibleModule"
  "LongParameterList"
  "ManualDispatch"
  "NestedIterators"
  "NilCheck"
  "PrimaDonnaMethod"
  "RepeatedConditional"
  "TooManyConstants"
  "TooManyInstanceVariables"
  "TooManyMethods"
  "TooManyStatements"
  "UncommunicativeMethodName"
  "UncommunicativeParameterName"
  "UncommunicativeVariableName"
  "UnusedParameters"
  "UtilityFunction"
  ```

---

.grayed-out[
  # 7. Static analysis tools - **code_metrics**
]

> rake stats is great for looking at statistics on your code, displaying things like KLOCs (thousands of lines of code) and your code to test ratio.

```bash
code_metrics
+----------------------+-------+-------+---------+---------+-----+-------+
| Name                 | Lines |   LOC | Classes | Methods | M/C | LOC/M |
+----------------------+-------+-------+---------+---------+-----+-------+
| Controllers          |  1382 |  1069 |      37 |     173 |   4 |     4 |
| Helpers              |   122 |    99 |       0 |      16 |   0 |     4 |
| Models               |  2077 |  1480 |      38 |     202 |   5 |     5 |
| Mailers              |     0 |     0 |       0 |       0 |   0 |     0 |
| Javascripts          |  7941 |  6199 |       0 |     921 |   0 |     4 |
| Libraries            |   706 |   537 |       6 |      82 |  13 |     4 |
+----------------------+-------+-------+---------+---------+-----+-------+
| Total                | 12228 |  9384 |      81 |    1394 |  17 |     4 |
+----------------------+-------+-------+---------+---------+-----+-------+
  Code LOC: 9384     Test LOC: 0     Code to Test Ratio: 1:0.0
```
---

.grayed-out[
  # 7. Static analysis tools - **rubocop**
]

> RuboCop is a Ruby static code analyzer. Out of the box it will enforce many of the guidelines outlined in the community [Ruby Style Guide](https://github.com/bbatsov/ruby-style-guide).

```bash
$ rubocop --format json app/models > rubocop.json
```
```json
{
  "metadata": {
  // omitted for brevity
  },
  "files": [
    {
      "path": "app/models/access_token.rb",
      "offenses": [
        {
          "severity": "convention",
          "message": "Prefer the new style validations `validates :column, presence: value` over `validates_presence_of`.",
          "cop_name": "Rails/Validation",
          "corrected": null,
          "location": {
            "line": 2,
            "column": 3,
            "length": 21
          }
        },
        {
          "severity": "convention",
          "message": "Do not use `Time.now` without zone. Use one of `Time.zone.now`, `Time.current`, `Time.now.in_time_zone`, `Time.now.utc`, `Time.now.getlocal`, `Time.now.iso8601`, `Time.now.jisx0301`, `Time.now.rfc3339`, `Time.now.to_i`, `Time.now.to_f` instead. (https://github.com/bbatsov/rails-style-guide#time, http://danilenko.org/2012/7/6/rails_timezones)",
          "cop_name": "Rails/TimeZone",
          "corrected": null,
          "location": {
            "line": 14,
            "column": 31,
            "length": 3
          }
        }
      ]
    },
```
---

.grayed-out[
  # 7. Static analysis tools - **rubocop**
]

- Again we could get all the severities for researching purposes:
  ```bash
  $ cat rubocop.json | jq '.files[].offenses[].severity' | sort | uniq -c
  7380 "convention"
   215 "warning"
  ```
---

.grayed-out[
  # 7. Static analysis tools - **unused**
]

```bash
lib/acts_as_customizable
     custom_field_values=        ...able/acts_as_customizable.rb    occurs once
     custom_value_for            ...able/acts_as_customizable.rb    occurs once

lib/acts_as_linkable
     find_linkable               .../link.rb                        occurs once
     find_links_for              .../acts_as_linkable.rb            occurs once
     find_links_for_linkable     .../link.rb                        occurs once
     linked_by_type              .../acts_as_linkable.rb            occurs once
     to_icon                     .../link.rb                        occurs once

lib/acts_as_noteable
     find_notes_by_user          .../acts_as_noteable.rb            occurs once
     find_notes_for              .../acts_as_noteable.rb            occurs once

lib/acts_as_urlnameable
     all_urlnames                ...ble/acts_as_urlnameable.rb      occurs once
     find_all_by_urlname         ...ble/acts_as_urlnameable.rb      occurs once

lib/acts_as_voteable
     users_who_voted             .../acts_as_voteable.rb            occurs once
     votes_against_percentage    .../acts_as_voteable.rb            occurs once
     votes_for_percentage        .../acts_as_voteable.rb            occurs once
     votes_not_sure_percentage   .../acts_as_voteable.rb            occurs once
```
---

# 8. Understand your tests
Try the following:
- Squint test your files
- Identity the [Subjet Under Test (SUT)](http://xunitpatterns.com/SUT.html)
- Understand the test setup and dependencies

And:
- Find out the test coverage
- Single Responsibility Principle also applies to tests

Further reading:
- There are [Test Smells](http://xunitpatterns.com/Test%20Smells.html)
---

# 9. Extract features
> Often times changing code will get just hard, consider extracting features into a new app

When:
- Lack of documentation or outdated
- Single test containing multiple SUT
- Low coverage
- Hard test setups
- Slow test suite

How:
- Create a high level collaborative test suite, check [Specification by Example](https://en.wikipedia.org/wiki/Specification_by_example)
- Bring enough code to make tests pass, iterate
---

# 10. Adopt a few core ideas
- Curiosity
- Assess risk
- Fear and courage
- Ensure safety harnesses
- Ask questions
- Experiment
- Take notes
- The boy scout rule

???
- Curiosity need to be your driven force
- You can not move without assessing Risk first
- Don't move without safety harnesses
- Ask questions to yourself, and everyone who may know about the system
- Fear is ok
---

# Thank you
Emmanuel as **chischaschos** at:
- [twitter](https://twitter.com/chischaschos)
- [github](https://github.com/chischaschos)
- [almost empty blog](https://chischaschos.github.io)
- [my email](mailto: larin.s931@gmail.com)