Dave Burt wrote:
> Hi Ruby Quiz,
> 
> Sorry, this is likely to be rather uninteresting, 

Actually, I find it very interesting ... but then, you have to consider 
my tastes. :)

> Refactoring 1: Merge Redundant Exception Handlers
> /You have a series of exception handling blocks with identical 
> handlers./
> *Merge the code into a single exception handling block.*
> 
> Thus:
> 
>   begin
>     foo
>   rescue Exception => e
>     barf e
>   end
>   begin
>     bar
>   rescue Exception => e
>     barf e
>   end
> 
> Becomes:
> 
>   begin
>     foo
>     bar
>   rescue Exception => e
>     barf e
>   end

You need to be careful here.  If 'barf' does not thrown an exception, 
but merely prints an error message of some kind, then the refactoring 
changes the behavior (which technically, makes it *not* a refactoring).

For example, from some (simplified) gem code I was working on just last 
week:

  def generate_docs
    begin
      generate_ri_docs
    rescue Exception => e
      puts "Problems in RI docs"
    end
    begin
      generate_rdocs
    rescue Exception => e
      puts "Problems in RDocs"
    end
  end

Merging the exception blocks in this case would mean that errors in 
generating the RI documents would skip the generation of the RDocs. 
Definitatly not a good thing.

--
-- Jim Weirich


> 
> Refactoring 2: Remove Unused Scope
> /You have a begin...end block that introduces a scope that is unused./
> *Remove the unused scope.*
> 
> Thus:
> 
>   def foo
>     begin
>       bar
>     rescue
>       baz
>     end
>   end
> 
> Becomes:
> 
>   def foo
>     bar
>   rescue
>     baz
>   end
> 
> Here is the refactored code:
> 
>    def service_main
>       @s.mount_proc("/yaml") { |req, res|
>          res['content-type'] = 'text/plain'
>          res.body << Gem::Cache.from_installed_gems(File.join(Gem.dir,
> "specifications")).to_yaml
>       }
> 
>       @s.mount_proc("/") { |req, res|
>          specs = []
>          specifications_dir = File.join(Gem.dir, "specifications")
>          Gem::Cache.from_installed_gems(specifications_dir).each do
> |path, spec|
>             specs << {
>                "name"           => spec.name,
>                "version"        => spec.version,
>                "full_name"      => spec.full_name,
>                "summary"        => spec.summary,
>                "rdoc_installed" =>
> Gem::DocManager.new(spec).rdoc_installed?,
>                "doc_path"       => ('/doc_root/' + spec.full_name +
> '/rdoc/index.html')
>             }
>          end
>          specs = specs.sort_by { |spec| [spec["name"].downcase,
> spec["version"]] }
>          template = TemplatePage.new(DOC_TEMPLATE)
>          res['content-type'] = 'text/html'
>          template.write_html_on(res.body, {"specs" => specs})
>       }
> 
>       {"/gems" => "/cache/", "/doc_root" => "/doc/"}.each do
> |mount_point, mount_dir|
>          @s.mount(mount_point, WEBrick::HTTPServlet::FileHandler,
> File.join(Gem.dir, mount_dir), true)
>       end
> 
>       @s.start
>    rescue Exception => e
>       File.open(@logfile,'w+'){ |f| f.puts "Start failed: #{e}" }
>       service_stop
>    end
> 
> Cheers,
> Dave


-- 
Posted via http://www.ruby-forum.com/.