Pull Request

Readme.md

@@ -0,0 +1,36 @@
+
+# Grupo 2
+# Integrantes
+  - Cristobal Martinez
+  - Carlos Aguirre
+
+## CodeSmell por Reek
+  Usando el comando rekk encontramos los siguientes code smells
+  ```
+    reek
+  ```
+
+  1.- Codigo Duplicado
+  2.- DuplicateMethod Call
+  3.- FeatureEnvy
+  4.- Too many Statements
+
+
+## CodeSmell Encontrados
+
+  - Feature Envy
+    - La clase Rental debiera ser la encargadda de calcular el costo de la pelicula y no la persona. Esto es un problema (Single Responsability de SOLID) ya que no podemos reutilizar el codigo y ademas genera mucha responsabilidad para la clase.
+
+  - Comments
+    - Comentarios Innecesarios
+      - Ej:
+        ```
+          # add frequent renter points
+          frequent_renter_points += 1
+          ```
+  - Long Method
+    - El metodo statement de la clase customer tiene mucha responsabilidades y es demasiado largo. Apenas cabe en una pagina.
+    - Es malo porque es un indicio que tiene muchas responsabilidades
+    - Funcionalidades debieran ser de la clase rental como por ejemplo:
+        - Calcular precio
+        - Calcular Bono

customer.rb

@@ -1,7 +1,7 @@
 # A customer of the store
 class Customer
   attr_reader :name
-  
+
   def initialize(name)
     @name = name
     @rentals = []
@@ -16,33 +16,13 @@ def statement
     result = "Rental Record for #{@name}\n"
 
     @rentals.each do |element|
-      this_amount = 0
-      # determine amounts for each line
-      case element.movie.price_code
-      when Movie::REGULAR
-        this_amount += 2
-        this_amount += (element.days_rented - 2) * 1.5 if element.days_rented > 2
-      when Movie::NEW_RELEASE
-        this_amount += element.days_rented * 3
-      when Movie::CHILDRENS
-        this_amount += 1.5
-        this_amount += (element.days_rented - 3) * 1.5 if element.days_rented > 3
-      end
-
-      # add frequent renter points
-      frequent_renter_points += 1
-
-      # add bonus for a two day new release rental
-      if element.movie.price_code == Movie::NEW_RELEASE && element.days_rented > 1
-          frequent_renter_points += 1
-      end
-
-      # show figures for this rental
-      result += "\t" + element.movie.title + "\t" + this_amount.to_s + "\n"
-      total_amount += this_amount
+      x, y, z = element.get_movie_statement
+      total_amount += x
+      frequent_renter_points += y
+      result += z
     end
 
-    # add footer lines
+    # Add footer lines
     result += "Amount owed is #{total_amount}\n"
     result += "You earned #{frequent_renter_points} frequent renter points"
     result

rental.rb

@@ -5,4 +5,39 @@ class Rental
   def initialize(movie, days_rented)
     @movie, @days_rented = movie, days_rented
   end
-end
+
+  def get_amount
+    this_amount = 0
+    case @movie.price_code
+    when Movie::REGULAR
+      this_amount += 2
+      this_amount += (@days_rented - 2) * 1.5 if @days_rented > 2
+    when Movie::NEW_RELEASE
+      this_amount += @days_rented * 3
+    when Movie::CHILDRENS
+      this_amount += 1.5
+      this_amount += (@days_rented - 3) * 1.5 if @days_rented > 3
+    end
+    return this_amount
+  end
+
+  def get_bono
+      if @movie.price_code == Movie::NEW_RELEASE && @days_rented > 1
+          return  1
+      end
+      return 0
+  end
+
+  def get_movie_statement
+    total_amount, frequent_renter_points = 0, 0
+    this_amount = self.get_amount
+    frequent_renter_points += 1
+    frequent_renter_points += self.get_bono
+
+    # show figures for this rental
+    result = "\t" + @movie.title + "\t" + this_amount.to_s + "\n"
+    total_amount += this_amount
+    return total_amount, frequent_renter_points, result
+  end
+
+end