From 163d18deca49d15fbb69a63d80ec919b88148910 Mon Sep 17 00:00:00 2001 From: Carlos Aguirre Date: Thu, 29 Sep 2016 13:04:53 -0300 Subject: [PATCH] Pull Request --- Readme.md | 36 ++++++++++++++++++++++++++++++++++++ customer.rb | 32 ++++++-------------------------- rental.rb | 37 ++++++++++++++++++++++++++++++++++++- 3 files changed, 78 insertions(+), 27 deletions(-) create mode 100644 Readme.md diff --git a/Readme.md b/Readme.md new file mode 100644 index 0000000..0cf7c97 --- /dev/null +++ b/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 diff --git a/customer.rb b/customer.rb index dbc96b1..c0bcfa5 100644 --- a/customer.rb +++ b/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 diff --git a/rental.rb b/rental.rb index e275201..e8a5fa1 100644 --- a/rental.rb +++ b/rental.rb @@ -5,4 +5,39 @@ class Rental def initialize(movie, days_rented) @movie, @days_rented = movie, days_rented end -end \ No newline at end of file + + 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