Your code is very brittle and hard-coded. Code is supposed to be as generic and flexible as possible so you could re-use it. A new chapter, a different layout, anything would require you to change the whole code.
Code is about doing work for you. Think generically - can this code be used for a different book? (I don't mean by you copying it, pasting it and changing it...)
You need to abstract it - what do you have here?
You have a title
, which is different than a chapter
. A chapter
has a page number
. There are some chapters in a book. The TOC needs to be laid out using some logic...
title
and chapter
are objects is your system, the layout logic is a method.
For example - I pass to a method called format_toc
a title, and a list of chapters, which (for simplicity) contain their name and their page number in an array. The method than lays out the TOC page according to the format in your code. Note that the data does not contain the words 'Chapter'
or 'page'
- they are part of the layout, not the data:
def format_toc(title, chapters)
line_width = 30
toc = chapters.each_with_index.map do |chapter, i|
"Chapter #{i}: #{chapter[0]}".ljust(line_width) + "page #{chapter[1]}".rjust(line_width)
end.join("\n")
"#{title.center(line_width*2)}\n#{toc}"
end
puts format_toc 'Table of Contents', [['Numbers', 1], ['Letters', 72], ['Variables', 118]]
#=> Table of Contents
#=>Chapter 1: Numbers page 1
#=>Chapter 2: Letters page 72
#=>Chapter 3: Variables page 118
But wait - you are still using hard-coded indexes!
True, [0]
and [1]
are still there, which looks a little "hard-coded", but these indexes refer to the data structure, not the data itself. A more elaborate design would likely use a hash to hold chapter data ({name: 'Numbers', page: 1}
), or even a class having two properties, but for the example above it shows the concept of encapsulation - as long as two modules know the language (=API), they can independently change things in their domain (wither adding chapters to the book, or change the layout), without affecting other parts of the code.
I also took the liberty of changing the structure of the input, as @Flambino remarked, but only because I assumed the choice of the structure of the input was not an outside constraint, but a design choice (it didn't make much sense otherwise).