-
Notifications
You must be signed in to change notification settings - Fork 0
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
add test and method #2
base: master
Are you sure you want to change the base?
Conversation
return true; | ||
} | ||
|
||
/*public static boolean isHexadecimal(byte[] bytes) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No es bueno dejar código comentado, quitalo por favor.
/* fin del metodo */ | ||
/* metodo 2 */ | ||
|
||
public static void main (String[] args) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
esta clase nunca se va a ejecutar por separado, por este motivo, no es bueno que tenga un main independiente.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fijate que en el ticket 505 lo que pide es modificar la función ISOUtil.hex2byte
Esta función tiene la siguiente firma:
byte[] hex2byte (String s)
Es decir, tu misión es controlar que los caracteres que están representados en el String, no contengan valores inválidos.
La función de controlar cada caracter, si es un símbolo válido de hexadecimal, tiene sentido. Sin embargo, esto hay que hacerlo en el contexto de un String (es el input de la función)
|
||
|
||
public static boolean isHexadecimal(byte[] bytes) { | ||
for (byte b : bytes) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
porque tenes 2 funciones "isHexadecimal" ?
En todo caso, podes hacer que esta que recibe byte[] le llame a la anterior. De otra manera, estarias teniendo dos implementaciones de lo mismo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
se me ocurrio hacerlo asi, pero voy a modificarlos sin problemas.
|
||
|
||
@Test | ||
public void TestsHex1() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Los nombres de los tests deberian tener nombres significativos. ejemplo:
TestConvertToHexValidInput
TestConvertToHexInvalidInput
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perfecto, los voy a correjir
@Test | ||
public void TestsHex3() { | ||
String p = null; | ||
ISOUtil.hex2byte(p); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lo que tenes que probar en un test que falla es justamente que lanza una excepción. Deberias en este caso agarrar la excepcion, y no hacer nada, porque es esperado que falle. Si no hay excepción, tenes que fallar.
Fijate como escribir junit con excepciones:
https://www.baeldung.com/junit-assert-exception
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok me voy a fijar y lanzar la exceptions
@@ -60,7 +81,7 @@ public void testAsciiToEbcdic() throws Throwable { | |||
@Test | |||
public void testAsciiToEbcdic1() throws Throwable { | |||
byte[] a = new byte[1]; | |||
byte[] result = ISOUtil.asciiToEbcdic(a); | |||
byte[] result = c.asciiToEbcdic(a); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
porque tocas esta linea ? Entiend que no deberias
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
La verdad que no toque, o derepente se movio mi puntero o algo, pero no toque fue sin querer perdon.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tenes que eliminar esto de los cambios.
|
||
if (s.isEmpty() || s.length() % 2 != 0) //verificar si el string es vacio | ||
{ | ||
throw new IllegalArgumentException("The string is empty or null"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
El mensaje tiene que ser más explicativo. El control está bueno, si no tenes un número par, entones tenes un string Hex inválido. Sin embargo, el mensaje no refleja eso.
@Test | ||
public void testIsHexadecimal_ValidInput_ThrowsExceptionWithMessage() { | ||
|
||
Exception exception = assertThrows(RuntimeException.class, () -> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Es mejor poner la excepción del tipo que estás esperando. En tu caso entiendo que es un IllegalArgumentException
public void testIsHexadecimal_ValidInput_ThrowsExceptionWithMessage() { | ||
|
||
Exception exception = assertThrows(RuntimeException.class, () -> { | ||
ISOUtil.hex2byte("1A2B3C"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
En este caso es un input válido, entonces no debería excepcionar, sino que el caso debería de comprobar que efectivamente logró convertir.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Entonces aca le saco nomas la excepcion o como seria Dani?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
asi es
No description provided.