HiveBrain v1.2.0
Get Started
← Back to all entries
patternrubyMinor

Song search app with dynamic tables

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
tablessearchappwithsongdynamic

Problem

I decided to make a Ruby app to search for songs, which uses the tinysong API. Things got really cryptic and weird in the view. I managed to get dynamic tables that adjust to data length, but I feel like there must be a much better way to accomplish this output.

```
require "rubygems"
require 'json'
require "httparty"

def search(search_terms)
search_string = search_terms.split(' ').join('+')
response = HTTParty.get("http://tinysong.com/s/#{search_string}?format=json&key=API_KEY_OMITTED_FOR_REVIEW")
song_hash = JSON.parse(response.body)
#puts "#{song_hash}"
end

def view_song_search(song_hash)
term_width = %x'tput cols'.to_i # Executes terminal
total_char_limit = term_width - 9
field_limit = total_char_limit / 3

# Sets width of each column to the width of it's highest data.
artist_width = album_width = song_width = 0
song_hash.each do |song|
if song["ArtistName"].length > artist_width then artist_width = song["ArtistName"].length end
if song["AlbumName"].length > album_width then album_width = song["AlbumName"].length end
if song["SongName"].length > song_width then song_width = song["SongName"].length end
end

# Sets the minimum column width equal to header width.
if artist_width total_char_limit
artist_width = album_width = song_width = field_limit
end

# Where things get cryptic.
printf("+%s+%s+%s+\n", "-" (artist_width + 1), "-" (album_width + 2), "-" * (song_width + 2))
printf("\e[7m|%-#{artist_width}s | %-#{album_width}s | %-#{song_width}s |\e[0m\n", "Artist", "Album", "Song")
printf("+%s+%s+%s+\n", "-" (artist_width + 1), "-" (album_width + 2), "-" * (song_width + 2))

song_hash.each do |song|
# Sets data of each column to not exceed column width.
if song["ArtistName"].length > artist_width ||
song["AlbumName"].length > album_width ||
song["SongName"].length > song_width
artist_name = (song["ArtistName"])[0...artist_width]
album_name = (song["AlbumNa

Solution

I'm going to assume that search returns a data structure like this:

data = [
  { 'ArtistName' => 'Crash Test Dummies',
    'AlbumName' => 'God Shuffled His Feet',
    'SongName' => 'Mmmm_Mmmmm...',
  },
  { 'ArtistName' => 'John Powell',
    'AlbumName' => 'How to Train Your Dragon',
    'SongName' => 'Test Drive',
  },
  …
]


In that case, using song_hash as the name of the parameter to view_song_search is misleading, since it's actually an array.

There's no reason why view_song_search needs to know anything about songs, or why it should be tied to artists, albums, and songs at all. You should write a generic table-formatting function, like this:

puts table(data, [['ArtistName', 'Artist'],
                  ['AlbumName', 'Album'],
                  ['SongName', 'Song']])


Here's an implementation of such a function. Remarks:

  • A large chunk of the code is the block in the middle to shrink overwide tables. I've tried to scale each column proportionally instead of giving one-third to each column.



  • The heading row resembles a data row in many ways, and should be handled using a common codepath.



  • The horizontal rules are printed many times; build it just once.



  • Use sprintf '%-*s', width, text to space-pad the text to the desired width.



def table(hashes, headings, width_limit=%x'tput cols'.to_i - 5)
  attrs = headings.map { |h| h.first }
  headings = Hash[headings]

  # Minimum widths to accommodate the heading and data of each column,
  # excluding leading space, trailing space, and vertical dividers
  col_widths = attrs.map do |attr|
    [headings[attr].size, hashes.map { |h| h[attr].size }.max].max
  end

  # If the desired width, including delimiters, exceeds the limit,
  # reduce column widths proportionally
  desired_width = col_widths.inject(:+) + 3 * col_widths.size + 1
  if desired_width > width_limit
    col_widths.each_with_index do |width, c|
      col_widths[c] = (width.to_f * width_limit / desired_width).to_i - 2
    end

    # Readjust last column for round-off errors
    desired_width = col_widths.inject(:+) + 3 * col_widths.size + 1
    col_widths[-1] += width_limit - desired_width
  end

  output = hrule = '+' + col_widths.map { |width| '-' * (width + 2) + '+' }.join
  ([headings] + hashes).each_with_index do |row, r|
    output += "\n"
    output += "\e[7m" if r == 0         # Highlight heading row
    output += '|'
    attrs.each_with_index do |attr, c|
      output += sprintf(' %-*s |', col_widths[c], row[attr][0...col_widths[c]])
    end
    output += "\e[0m" if r == 0         # Turn off highlight
    output += "\n" + hrule
  end
  output
end

Code Snippets

data = [
  { 'ArtistName' => 'Crash Test Dummies',
    'AlbumName' => 'God Shuffled His Feet',
    'SongName' => 'Mmmm_Mmmmm...',
  },
  { 'ArtistName' => 'John Powell',
    'AlbumName' => 'How to Train Your Dragon',
    'SongName' => 'Test Drive',
  },
  …
]
puts table(data, [['ArtistName', 'Artist'],
                  ['AlbumName', 'Album'],
                  ['SongName', 'Song']])
def table(hashes, headings, width_limit=%x'tput cols'.to_i - 5)
  attrs = headings.map { |h| h.first }
  headings = Hash[headings]

  # Minimum widths to accommodate the heading and data of each column,
  # excluding leading space, trailing space, and vertical dividers
  col_widths = attrs.map do |attr|
    [headings[attr].size, hashes.map { |h| h[attr].size }.max].max
  end

  # If the desired width, including delimiters, exceeds the limit,
  # reduce column widths proportionally
  desired_width = col_widths.inject(:+) + 3 * col_widths.size + 1
  if desired_width > width_limit
    col_widths.each_with_index do |width, c|
      col_widths[c] = (width.to_f * width_limit / desired_width).to_i - 2
    end

    # Readjust last column for round-off errors
    desired_width = col_widths.inject(:+) + 3 * col_widths.size + 1
    col_widths[-1] += width_limit - desired_width
  end

  output = hrule = '+' + col_widths.map { |width| '-' * (width + 2) + '+' }.join
  ([headings] + hashes).each_with_index do |row, r|
    output += "\n"
    output += "\e[7m" if r == 0         # Highlight heading row
    output += '|'
    attrs.each_with_index do |attr, c|
      output += sprintf(' %-*s |', col_widths[c], row[attr][0...col_widths[c]])
    end
    output += "\e[0m" if r == 0         # Turn off highlight
    output += "\n" + hrule
  end
  output
end

Context

StackExchange Code Review Q#85216, answer score: 2

Revisions (0)

No revisions yet.