Project-wise refactoring to follow Ameba

spec/config_spec.cr

@@ -2,7 +2,7 @@ require "./spec_helper"
 
 describe Config do
   it "creates config if it does not exist" do
-    with_default_config do |config, logger, path|
+    with_default_config do |_, _, path|
       File.exists?(path).should be_true
     end
   end

spec/mangadex_spec.cr

@@ -4,7 +4,7 @@ include MangaDex
 
 describe Queue do
   it "creates DB at given path" do
-    with_queue do |queue, path|
+    with_queue do |_, path|
       File.exists?(path).should be_true
     end
   end

spec/spec_helper.cr

@@ -28,9 +28,9 @@ def get_tempfile(name)
   if path.nil? || !File.exists? path
     file = File.tempfile name
     State.set name, file.path
-    return file
+    file
   else
-    return File.new path
+    File.new path
   end
 end
 
@@ -43,7 +43,7 @@ def with_default_config
 end
 
 def with_storage
-  with_default_config do |config, logger|
+  with_default_config do |_, logger|
     temp_db = get_tempfile "mango-test-db"
     storage = Storage.new temp_db.path, logger
     clear = yield storage, temp_db.path
@@ -54,7 +54,7 @@ def with_storage
 end
 
 def with_queue
-  with_default_config do |config, logger|
+  with_default_config do |_, logger|
     temp_queue_db = get_tempfile "mango-test-queue-db"
     queue = MangaDex::Queue.new temp_queue_db.path, logger
     clear = yield queue, temp_queue_db.path

spec/storage_spec.cr

@@ -2,7 +2,7 @@ require "./spec_helper"
 
 describe Storage do
   it "creates DB at given path" do
-    with_storage do |storage, path|
+    with_storage do |_, path|
       File.exists?(path).should be_true
     end
   end

src/library.cr

@@ -27,12 +27,10 @@ class Entry
     @encoded_title = URI.encode @title
     @size = (File.size path).humanize_bytes
     file = Zip::File.new path
-    @pages = file.entries
+    @pages = file.entries.count do |e|
-      .select { |e|
+      ["image/jpeg", "image/png"].includes? \
-        ["image/jpeg", "image/png"].includes? \
+        MIME.from_filename? e.filename
-          MIME.from_filename? e.filename
+    end
-      }
-      .size
     file.close
     @id = storage.get_id @zip_path, false
     @cover_url = "/api/page/#{@title_id}/#{@id}/1"
@@ -178,15 +176,13 @@ class Title
   #   Entry.new would throw, so we use this method to check before
   #   constructing Entry
   private def valid_zip(path : String)
-    begin
+    file = Zip::File.new path
-      file = Zip::File.new path
+    file.close
-      file.close
+    true
-      return true
+  rescue
-    rescue
+    @logger.warn "File #{path} is corrupted or is not a valid zip " \
-      @logger.warn "File #{path} is corrupted or is not a valid zip " \
+                 "archive. Ignoring it."
-                   "archive. Ignoring it."
+    false
-      return false
-    end
   end
 
   def get_entry(eid)
@@ -249,7 +245,6 @@ class Title
   end
 
   def load_percetage(username, entry)
-    info = TitleInfo.new @dir
     page = load_progress username, entry
     entry_obj = @entries.find { |e| e.title == entry }
     return 0.0 if entry_obj.nil?

src/mangadex/api.cr

@@ -52,29 +52,27 @@ module MangaDex
     end
 
     def parse_json(obj, lang)
-      begin
+      parse_strings_from_json ["lang_code", "title", "volume",
-        parse_strings_from_json ["lang_code", "title", "volume",
+                               "chapter"]
-                                 "chapter"]
+      language = lang[@lang_code]?
-        language = lang[@lang_code]?
+      @language = language if language
-        @language = language if language
+      @time = Time.unix obj["timestamp"].as_i
-        @time = Time.unix obj["timestamp"].as_i
+      suffixes = ["", "_2", "_3"]
-        suffixes = ["", "_2", "_3"]
+      suffixes.each do |s|
-        suffixes.each do |s|
+        gid = obj["group_id#{s}"].as_i
-          gid = obj["group_id#{s}"].as_i
+        next if gid == 0
-          next if gid == 0
+        gname = obj["group_name#{s}"].as_s
-          gname = obj["group_name#{s}"].as_s
+        @groups << {gid, gname}
-          @groups << {gid, gname}
-        end
-        @full_title = @title
-        unless @chapter.empty?
-          @full_title = "Ch.#{@chapter} " + @full_title
-        end
-        unless @volume.empty?
-          @full_title = "Vol.#{@volume} " + @full_title
-        end
-      rescue e
-        raise "failed to parse json: #{e}"
       end
+      @full_title = @title
+      unless @chapter.empty?
+        @full_title = "Ch.#{@chapter} " + @full_title
+      end
+      unless @volume.empty?
+        @full_title = "Vol.#{@volume} " + @full_title
+      end
+    rescue e
+      raise "failed to parse json: #{e}"
     end
   end
 
@@ -108,12 +106,10 @@ module MangaDex
     end
 
     def parse_json(obj)
-      begin
+      parse_strings_from_json ["cover_url", "description", "title", "author",
-        parse_strings_from_json ["cover_url", "description", "title", "author",
+                               "artist"]
-                                 "artist"]
+    rescue e
-      rescue e
+      raise "failed to parse json: #{e}"
-        raise "failed to parse json: #{e}"
-      end
     end
   end
 
@@ -146,7 +142,7 @@ module MangaDex
           chapter = Chapter.new k, v, manga, @lang
           manga.chapters << chapter
         end
-        return manga
+        manga
       rescue
         raise "Failed to parse JSON"
       end
@@ -195,7 +191,7 @@ module MangaDex
       manga = self.get_manga manga_id
       chapter = manga.chapters.find { |c| c.id == id }.not_nil!
       self.get_chapter chapter
-      return chapter
+      chapter
     end
   end
 end

src/mangadex/downloader.cr

@@ -52,7 +52,7 @@ module MangaDex
     def self.from_query_result(res : DB::ResultSet)
       job = Job.allocate
       job.parse_query_result res
-      return job
+      job
     end
 
     def initialize(@id, @manga_id, @title, @manga_title, @status, @time)
@@ -120,7 +120,7 @@ module MangaDex
         rescue
         end
       end
-      return job
+      job
     end
 
     # Push an array of jobs into the queue, and return the number of jobs
@@ -203,7 +203,7 @@ module MangaDex
           Job.from_query_result rs
         end
       end
-      return jobs
+      jobs
     end
 
     def add_success(job : Job)
@@ -338,7 +338,7 @@ module MangaDex
             @logger.error msg
           end
         end
-        fail_count = page_jobs.select { |j| !j.success }.size
+        fail_count = page_jobs.count { |j| !j.success }
         @logger.debug "Download completed. " \
                       "#{fail_count}/#{page_jobs.size} failed"
         writer.close

src/mango.cr

@@ -7,7 +7,7 @@ VERSION = "0.2.5"
 
 config_path = nil
 
-parser = OptionParser.parse do |parser|
+OptionParser.parse do |parser|
   parser.banner = "Mango e-manga server/reader. Version #{VERSION}\n"
 
   parser.on "-v", "--version", "Show version" do
@@ -31,7 +31,7 @@ library = Library.new config.library_path, config.scan_interval, logger, storage
 queue = MangaDex::Queue.new config.mangadex["download_queue_db_path"].to_s,
   logger
 api = MangaDex::API.new config.mangadex["api_url"].to_s
-downloader = MangaDex::Downloader.new queue, api, config.library_path,
+MangaDex::Downloader.new queue, api, config.library_path,
   config.mangadex["download_wait_seconds"].to_i,
   config.mangadex["download_retries"].to_i, logger
 

src/routes/admin.cr

@@ -26,77 +26,73 @@ class AdminRouter < Router
 
     post "/admin/user/edit" do |env|
       # creating new user
-      begin
+      username = env.params.body["username"]
-        username = env.params.body["username"]
+      password = env.params.body["password"]
-        password = env.params.body["password"]
+      # if `admin` is unchecked, the body hash
-        # if `admin` is unchecked, the body hash
+      #   would not contain `admin`
-        #   would not contain `admin`
+      admin = !env.params.body["admin"]?.nil?
-        admin = !env.params.body["admin"]?.nil?
 
-        if username.size < 3
+      if username.size < 3
-          raise "Username should contain at least 3 characters"
+        raise "Username should contain at least 3 characters"
-        end
+      end
-        if (username =~ /^[A-Za-z0-9_]+$/).nil?
+      if (username =~ /^[A-Za-z0-9_]+$/).nil?
-          raise "Username should contain alphanumeric characters " \
+        raise "Username should contain alphanumeric characters " \
-                "and underscores only"
+              "and underscores only"
-        end
+      end
+      if password.size < 6
+        raise "Password should contain at least 6 characters"
+      end
+      if (password =~ /^[[:ascii:]]+$/).nil?
+        raise "password should contain ASCII characters only"
+      end
+
+      @context.storage.new_user username, password, admin
+
+      env.redirect "/admin/user"
+    rescue e
+      @context.error e
+      redirect_url = URI.new \
+        path: "/admin/user/edit",
+        query: hash_to_query({"error" => e.message})
+      env.redirect redirect_url.to_s
+    end
+
+    post "/admin/user/edit/:original_username" do |env|
+      # editing existing user
+      username = env.params.body["username"]
+      password = env.params.body["password"]
+      # if `admin` is unchecked, the body hash would not contain `admin`
+      admin = !env.params.body["admin"]?.nil?
+      original_username = env.params.url["original_username"]
+
+      if username.size < 3
+        raise "Username should contain at least 3 characters"
+      end
+      if (username =~ /^[A-Za-z0-9_]+$/).nil?
+        raise "Username should contain alphanumeric characters " \
+              "and underscores only"
+      end
+
+      if password.size != 0
         if password.size < 6
           raise "Password should contain at least 6 characters"
         end
         if (password =~ /^[[:ascii:]]+$/).nil?
           raise "password should contain ASCII characters only"
         end
-
-        @context.storage.new_user username, password, admin
-
-        env.redirect "/admin/user"
-      rescue e
-        @context.error e
-        redirect_url = URI.new \
-          path: "/admin/user/edit",
-          query: hash_to_query({"error" => e.message})
-        env.redirect redirect_url.to_s
       end
-    end
 
-    post "/admin/user/edit/:original_username" do |env|
+      @context.storage.update_user \
-      # editing existing user
+        original_username, username, password, admin
-      begin
-        username = env.params.body["username"]
-        password = env.params.body["password"]
-        # if `admin` is unchecked, the body hash would not contain `admin`
-        admin = !env.params.body["admin"]?.nil?
-        original_username = env.params.url["original_username"]
 
-        if username.size < 3
+      env.redirect "/admin/user"
-          raise "Username should contain at least 3 characters"
+    rescue e
-        end
+      @context.error e
-        if (username =~ /^[A-Za-z0-9_]+$/).nil?
+      redirect_url = URI.new \
-          raise "Username should contain alphanumeric characters " \
+        path: "/admin/user/edit",
-                "and underscores only"
+        query: hash_to_query({"username" => original_username, \
-        end
+                                 "admin" => admin, "error" => e.message})
-
+      env.redirect redirect_url.to_s
-        if password.size != 0
-          if password.size < 6
-            raise "Password should contain at least 6 characters"
-          end
-          if (password =~ /^[[:ascii:]]+$/).nil?
-            raise "password should contain ASCII characters only"
-          end
-        end
-
-        @context.storage.update_user \
-          original_username, username, password, admin
-
-        env.redirect "/admin/user"
-      rescue e
-        @context.error e
-        redirect_url = URI.new \
-          path: "/admin/user/edit",
-          query: hash_to_query({"username" => original_username, \
-                                   "admin" => admin, "error" => e.message})
-        env.redirect redirect_url.to_s
-      end
     end
 
     get "/admin/downloads" do |env|

src/util.cr

@@ -42,7 +42,7 @@ def request_path_startswith(env, ary)
       return true
     end
   end
-  return false
+  false
 end
 
 def is_numeric(str)