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

Critique some Ruby + RSpec Code for a SERP checker

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

Problem

I found a SERP checker written in PHP and I decided, in order to better learn to program, I would re-write it in Ruby.

All I have written so far takes a list of keywords that a user inputs and cleans the list and turns each keyword into a url for Google search.

Here is the code:

require 'sinatra'
require 'rspec'
get '/serp_checker' do
  "
  Keyword
  
  URL
  
  
  "
end

def clean_up_keywords(str)
  str.gsub("\n", ",").delete("\r").split(',')
end

def clean_up_list(arr)
  arr.reject(&:empty?).each(&:lstrip!)
end

def make_strings_url_friendly(arr)
  arr.each do |e|
    e.gsub!(" ", "+")
  end
end

def make_urls(arr)
  arr.map {|e| "http://www.google.com/search?num=100&q=" + e}
end

post '/ranked' do
  dirty_list = clean_up_keywords(params[:keyword])
  clean_list = clean_up_list(dirty_list)
  url_ready_list = make_strings_url_friendly(clean_list)
  url_list = make_urls(url_ready_list)
end


Here is the spec:

```
require_relative '../lib/rankypanky.rb'

describe "#clean_up_keywords" do
it "should push items separated by a newline into array as separate items" do
clean_up_keywords("apples\noranges").should == ["apples", "oranges"]
end
it "should delete all \r chars" do
clean_up_keywords("Chat\r").should == ["Chat"]
end
it "should push items separated by commas into an array" do
clean_up_keywords("Chat, Meta, About").should == ["Chat", " Meta", " About"]
end
it "should push items separated ONLY by a \s char into array as one item" do
clean_up_keywords("New York, apples").should == ["New York", " apples"]
end
end

describe "#clean_up_list" do
it "should not include empty strings/items in array" do
clean_up_list(["apples", "", "oranges"]).should == ["apples", "oranges"]
end
it "should remove any leading white space from items in array" do
clean_up_list([" oranges", "apples"]).should == ["oranges", "apples"]
end
end

describe "#make_strings_url_friendly" do
it "should replace /s with a +" do
make_strings_url

Solution

I see you haven't got any response on this for a long while, so I'll give it a try.

I don't really see anything to pick on when it comes to style or technique. To me this looks very good. A few questions though:

  • why do you require "rspec" in the controller code? Shouldn't this be in the test code instead?



  • You don't do any attempt to sanitize or verify that the code works with non-english characters, or other characters that may cause trouble. What if someone type "tørris&message=alert("Hoy there");" in the input form? (I guess google will sanitize it, but it would probably be better to do it before passing the url to them.)

Context

StackExchange Code Review Q#2358, answer score: 3

Revisions (0)

No revisions yet.