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

Code improvement #82

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

Code improvement #82

wants to merge 101 commits into from

Conversation

javrr-ui
Copy link
Owner

@javrr-ui javrr-ui commented Apr 8, 2021

Resueltos muchos problemas pequeños detectados por el analizador de código estático, también refactoricé código duplicado.

@javrr-ui javrr-ui added the enhancement New feature or request label Apr 8, 2021
@javrr-ui javrr-ui self-assigned this Apr 8, 2021
Copy link
Collaborator

@javatlacati javatlacati left a comment

Choose a reason for hiding this comment

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

esos puntos de momento

@@ -60,7 +59,7 @@ public Config() {

public boolean loadDefaultSettings() {
//obtiene el archivo default.properties
InputStream defaultFile = Main.class.getClassLoader().getResourceAsStream("default.properties");
InputStream defaultFile = GameRunner.class.getClassLoader().getResourceAsStream("default.properties");
Copy link
Collaborator

Choose a reason for hiding this comment

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

No se cierran los flujos en caso de error lo que peude corromper los archivos de configuración.

@@ -262,7 +262,8 @@ public int opciones() {
return opc;
}

public boolean esOpcionValida(String entrada, int cantidadOpciones) {
public boolean esOpcionValida(String ent, int cantidadOpciones) {
String entrada = ent;
Copy link
Collaborator

Choose a reason for hiding this comment

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

esta es una reasignación inútil.

try {
userProperties.store(new FileWriter(configFilePath), null);
} catch (IOException e) {
System.out.println("Couln't save settings: " + e);
Copy link
Collaborator

Choose a reason for hiding this comment

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

esto es hacer más grande el issue #60 favor de corregirlo primero.

for(String argumentos: args){
if(argumentos.equals("no-gui")){

if (args != null && args.length > 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

es mejor poner las constantes del lado izquierdo de una comparación


if (args != null && args.length > 0) {
for (String argumentos : args) {
if (argumentos.equals("no-gui")) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

constantes del lado izquierdo de la comparación

return color.getRed() + "," + color.getGreen() + "," + color.getBlue();
}

public static Color getColorXD(String propertyValue) {
public static Color getColorFromRGB(String propertyValue) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Los métodos públicos deben llevar documentación

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

Successfully merging this pull request may close these issues.

3 participants