Instantly share code, notes, and snippets.

Embed
What would you like to do?
SCORE_OF = {1=>100, 2=>0, 3=>0, 4=>0, 5=>50, 6=>0}
SCORE_OF_A_SET_OF_THREE = {1=>1000, 2=>200, 3=>300, 4=>400, 5=>500, 6=>600}
def score(dice)
score = 0
(1..6).each{ |number|
score += score_of_set_of(number, dice)
}
return score
end
def score_of_set_of(number, dice)
score = 0
score += score_of_sets_of_three(number, dice)
score += score_of_set_of_less_than_three(number, dice)
return score
end
def number_of(number, dice)
return dice.select { |element| element == number}.size
end
def score_of_sets_of_three(number, dice)
return SCORE_OF_A_SET_OF_THREE[number] * number_of_sets_of_three(number_of(number,dice))
end
def score_of_set_of_less_than_three(number, dice)
return SCORE_OF[number] * number_of_no_set_of_three(number_of(number, dice))
end
def number_of_sets_of_three(number_of_elements)
number_of_elements / 3
end
def number_of_no_set_of_three(number_of_elements)
number_of_elements % 3
end
@plagelao

This comment has been minimized.

plagelao commented Dec 10, 2010

Hola Jesus :D A mi código también le han dado lo suyo hoy :) Ahora le toca al tuyo :P

  1. Al principio del todo, en el método score, hablas de score, dice, sum, item, number y element. ¿Alguno de esos términos son "sinónimos" en tu modelo? Si la respuesta es sí, elige un único nombre para el concepto.
    2.¿Por qué es necesario conocer el array_of_elements (sea lo que sea eso)? ¿No nos bastaria con conocer el número de veces que aparece el elemento? Un entero es más simple que un array, y tenemos que intentar que nuestro código sea lo más simple posible (que conste que a mi esto me cuesta mogollón). Si te fijas, la mayoría de los métodos que usan el array lo hacen para pedirle el tamaño.
    3.En ruby, la convención para los nombres de métodos es separar las palabras con _ y no capitalizar.

Si mis preguntas tienen algún sentido y refactorizas el gist, luego podemos seguir mirando el código :P
Dicho esto, mi código para este problema es muy malo, tengo que hacer los koans otra vez :D

@jjballano

This comment has been minimized.

Owner

jjballano commented Dec 10, 2010

He cambiado algunas cosas de las que me has dicho y otras las hago mañana :)
Voy a dejar el código antiguo y voy a ir modificando el nuevo de cara a ver como estaba y como ha quedado después de varias refactorizaciones.
Gracias por los comentarios!

@plagelao

This comment has been minimized.

plagelao commented Dec 10, 2010

Mola :D

@jjballano

This comment has been minimized.

Owner

jjballano commented Dec 10, 2010

Me he quedado con la cosa de que tenía que cambiarlo y al final me he decidido a hacerlo ahora... ¿qué te parece ahora?

@plagelao

This comment has been minimized.

plagelao commented Dec 10, 2010

Me entero de algo más, aunque tengo algunas dudas aún :P

  1. En number_of_elements creo que hay algo raro con los nombres. Hablas de dice, item, element y number mientras que en el resto del código sólo aparecen dice, number y element. Creo que el nombre de item significa element ¿no?
  2. score_of_groups_of_three y number_of_non_groups_with tienen que ver con element pero su nombre no lo indica ¿No es eso algo incoherente con respecto al resto de métodos que tienen que ver con element?

Un saludo :)

@kinisoftware

This comment has been minimized.

kinisoftware commented Dec 11, 2010

Buenas! :)
Siguiendo con lo que dice Alberto, quizás, debería aparecer por algún sitio el término "repeticiones" (recurrence maybe) mejor que elementos, por ejemplo aquí: "score_of_three_elements_of(number)" me da la sensación que te ha obligado el poner "elements" no querer poner otra vez "numbers". Además a mi también me parece que item significa element.
Otra cosa, y no sé Ruby, ¿el "dice" no se podría guardar en una variable o algo así para que sea accedido por otras funciones que lo usan? Creo que eso podría simplificar y mejorar la semántica de algunos métodos.

Un saludo!

@jjballano

This comment has been minimized.

Owner

jjballano commented Dec 11, 2010

He puesto como dice Kini el "dice" como global, quizás tenga sentido... no lo se, quizás en la próxima iteración lo quite ;)
Un repaso a los nombres... he usado los mismos que en el enunciado. En vez de "group" en el enunciado dice "set of the numbers". No usa la palabra elements así que la he eliminado. Y no se que más cosas...
Me queda un método llamado number_of(number) no me gusta... ¿ideas?
Por cierto, aunque supongo que lo sabreis, pero los test siempre pasan :)

@plagelao

This comment has been minimized.

plagelao commented Dec 11, 2010

Creo que, ahora que lo veo más claro, el método "score" hace demasiadas cosas. Además de sumar la puntuación para cada cara del dado, calcula la puntuación para cada cara del dado. Yo intentaría separar esas responsabilidades (una clase "DiceSide" o algo así, que calcule el resultado para cada cara?
A mi, personalmente, no me gusta lo de "dice" como global. Creo que si aparece el "DiceSide", en realidad el argumento que sobra en los métodos es el "number", no el "dice".
También creo que este es un paso muuuy largo, y que seguro que se puede hacer en pasos chiquitines, pero no se como :(

@jjballano

This comment has been minimized.

Owner

jjballano commented Dec 11, 2010

Yo veo eso que dices y esta condición:

if (number_of(number) > 0) and (number == 5 or number == 1)

Me chirría bastante también. Hay que seguir dándole vueltas.
Lo que no termino de ver es lo de que sobre number, te refieres a que tendría que estar como global y que todos leyeran de ahí? Me parece que complica la lectura del código no?

@plagelao

This comment has been minimized.

plagelao commented Dec 11, 2010

Con lo de que sobra number me refiero a que, probablemente, sería una propiedad de la clase "DiceSide". La cara del dado sabe que numero tiene ¿no? :P

@jjballano

This comment has been minimized.

Owner

jjballano commented Dec 11, 2010

Ah vale, no había entendido lo de que fuera una clase... i'll think about it!

@kinisoftware

This comment has been minimized.

kinisoftware commented Dec 11, 2010

De acuerdo con lo que decís. El tema de poner cosas globales a mi tampoco me gusta, era una propuesta/sugerencia a tener en cuenta pero no conozco el lenguaje ni la forma de poder hacerlo. Quizás, siguiendo con lo que dice Alberto, estaría bien una clase DiceSide con un "value" o "number" (DiceSide.number).
Mi propuesta para "number_of(number)" sería "recurrence_of(number)"... o algo así ;) pero le falta semántica (reaparición del número, donde?) un "recurrence_of_dice_side_in_throw()" o "recurrence_in_throw_of(diceside)".... venga, quién da más! :D

@jjballano

This comment has been minimized.

Owner

jjballano commented Dec 11, 2010

Siguiente aproximación hecha a partir de alguno de vuestros comentarios. Se reduce el código y parece más legible. Me siguen faltando cosas que decís, como tema naming, el DiceSice (que no termino de ver cómo meterlo sin liarla más), el método score parece que no cumple con una única responsabilidad...
De momento ya me he quitado el if ese que no me gustaba :)

@jjballano

This comment has been minimized.

Owner

jjballano commented Dec 11, 2010

De todas formas, estoy viendo que hay algo que no cambié porque venía así, pero es que venía mal... en el método score, el parámetro se llama "dice" cuando realmente no es un dado, es el resultado de haber lanzado el dado X veces... seguramente cambiando eso ya vea mejor como meter una clase Dice o DiceSide o ya se verá...

@kinisoftware

This comment has been minimized.

kinisoftware commented Dec 11, 2010

Yay! Me gusta más este código! :D

Es cierto lo del "dice", realmente es una "throw" (tirada... que lo pone en el diccionario mayormente :P). Igual así se puede sacar más semántica, quizás cada tirada está formada por "diceside" o "dicevalue"... podría no hacerse como una clase claro pero entonces no estamos siendo todo lo orientado a objetos que podríamos ;) (esto lo aprendí bien en el curso de TDD con Alejandro, no había primitivos salvo en las clases más bajas, en este caso DiceSide tiene un primitivo "value" :))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment