Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Java-Nerdery-ChallengeV2 - Extra-challenge - Luis Moroco #4

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

luismoroco
Copy link

@luismoroco luismoroco commented Nov 20, 2024

  • Creé una clase llamada Weather que se mapea usando Jackson desde el array de objetos en formato JSON. Estos se instancian una a una, lo cual, en manejable por la memoria. Además cuenta con un verificador de Fields para evitar procesar los objetos con datos faltantes/nulos.
  • Creé una clase llamada Report que contiene un Mapa de Fields que son propiedades de Weather que tienen como valor instancias de Variable que contiene los datos minimum, average y maximun; además de una función para actualizarlos y otra para mostrarlo en el formato requerido.
  • Creé una clase abstracta llamada MeteorologyReport con dos métodos process y show para procesar los datos de cada instancia de Weather. Esta tiene dos subclases AggregatedMeteorologyReport (Generar un reporte de todo el conjunto de datos) y DailyMeteorologyReport (separarlos por fechas DÍAS). Aggregate instancia un Report y obtiene las Fields de las instancias de Weather para utilizar la función Variable.update() para actualizar los valores tomando el nombre de las propiedades. En cuanto a DailyMeteorologyReport crea un HashMap donde Weather.time será la key y tendrá asociado una instancia de AggregatedMeteorologyReport también implementa process , pero verifica que exista la fecha y si no la crea y utiliza AggregatedMeteorologyReport.process para actualizar su propio reporte. Para intercambiar las políticas de procesamiento, solo debe intercambiarse la línea 13 del archivo ExtraChallenge final static MeteorologyReport meteorologyReport = new POLICY; .

Copy link
Collaborator

@sbobadilla8 sbobadilla8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall you have some interesting approaches, yet I think you overcomplicated yourself in several places. Follow the KISS principle.

@sbobadilla8
Copy link
Collaborator

  • Creé una clase llamada Weather que se mapea usando Jackson desde el array de objetos en formato JSON. Estos se instancian una a una, lo cual, en manejable por la memoria. Además cuenta con un verificador de Fields para evitar procesar los objetos con datos faltantes/nulos.
  • Creé una clase llamada Report que contiene un Mapa de Fields que son propiedades de Weather que tienen como valor instancias de Variable que contiene los datos minimum, average y maximun; además de una función para actualizarlos y otra para mostrarlo en el formato requerido.
  • Creé una clase abstracta llamada MeteorologyReport con dos métodos process y show para procesar los datos de cada instancia de Weather. Esta tiene dos subclases AggregatedMeteorologyReport (Generar un reporte de todo el conjunto de datos) y DailyMeteorologyReport (separarlos por fechas DÍAS). Aggregate instancia un Report y obtiene las Fields de las instancias de Weather para utilizar la función Variable.update() para actualizar los valores tomando el nombre de las propiedades. En cuanto a DailyMeteorologyReport crea un HashMap donde Weather.time será la key y tendrá asociado una instancia de AggregatedMeteorologyReport también implementa process , pero verifica que exista la fecha y si no la crea y utiliza AggregatedMeteorologyReport.process para actualizar su propio reporte. Para intercambiar las políticas de procesamiento, solo debe intercambiarse la línea 13 del archivo ExtraChallenge final static MeteorologyReport meteorologyReport = new POLICY; .

Also this should be in English.

Copy link
Collaborator

@sbobadilla8 sbobadilla8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still not fully understanding the way you handle the aggregated and daily report. Maybe we can check that in the office tomorrow.

…alues. Simplify winner verification in winningHand
Copy link
Collaborator

@sbobadilla8 sbobadilla8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know you are still making changes in the extra challenge, but everything else looks way better now.

return cards.stream()
.sorted(Comparator.reverseOrder())
.limit(2)
.reduce(0, (a, b) -> a * 10 + b);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

Comment on lines +123 to +127
BigInteger ownPower = BigInteger.ONE;
for (int j = 1; j <= i; ++j) {
ownPower = ownPower.multiply(BigInteger.valueOf(i));
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check modPow or pow

}

this.sum += value;
++this.count;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this.count += 1 is more understandable IMO

@quebin31
Copy link
Collaborator

Parsing JSON into a Java class shouldn't need to use reflection from your end, reconsider your approach

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants