No es necesario que las funciones del tda tengan el nombre del tda al comenzar, ademas en este caso queda muy largo y no se lee claramente.

Ademas la mescla de snake_case y CamelCase no es correcta, C suele usar la convesion de snake_case

https://github.com/joaquinfontela/tp1-taller/blob/d73293bbfda64a3e6926602c4680a5b133f9bdb0/client_commandValidator.c#L3

En cuanto a esto, en la clase nos habian dicho que debiamos utilizar el nombre del TDA al principio del metodo, y asi lo habia visto tambien en otras materias. Con respecto al nombre del metodo, en Algoritmos III nos habian explicado que preferian un nombre largo pero entendible, que viceversa.

En cuanto a snake_case o Camel Case, he visto como vos decis que en C se utiliza mas snake, sin embargo estoy muy acostumbrado a utilizar Camel, y me resulta mas comodo y entendible. En los unicos lugares donde utilize snake fue cuando debia utilizar el nombre de un TDA, ya que nos dijeron que debiamos nombrarlo como "_t"

joaquinfontela commented 3 years ago

No usar buffers de longitud constante https://github.com/joaquinfontela/tp1-taller/blob/d73293bbfda64a3e6926602c4680a5b133f9bdb0/client_main.c#L29

Con esto te referis a que deberia hacer algo del estilo:

"

define BUFFER_LEN 70

(...) char buffer[BUFFER_LEN]; " ?

joaquinfontela commented 3 years ago

No hagas esto, inicializalo sin definirlo https://github.com/joaquinfontela/tp1-taller/blob/d73293bbfda64a3e6926602c4680a5b133f9bdb0/client_socket.c#L7

Que deberia hacer aca? Ya que si unicamente lo declaro ("socket newSocket;") me lanza error. Pero no se como inicializar un struct sin definirlo.

joaquinfontela commented 3 years ago

En vez de unsigned int podes usar los unit32_t, etc https://github.com/joaquinfontela/tp1-taller/blob/d73293bbfda64a3e6926602c4680a5b133f9bdb0/common_cesar.c#L24

Unisigned char (? https://github.com/joaquinfontela/tp1-taller/blob/d73293bbfda64a3e6926602c4680a5b133f9bdb0/common_cesar.c#L25

Aca ya he hecho los reemplazos de "unsigned int" por "uint_32", aunque no comprendo la ventaja. Por otro lado, que es lo que esta mal cuando declaro "current char" como "unsigned char"? Estoy extrayendo un caracter de la string.

joaquinfontela commented 3 years ago

Muy similar este codigo, busca la forma de achicarlo

https://github.com/joaquinfontela/tp1-taller/blob/d73293bbfda64a3e6926602c4680a5b133f9bdb0/common_encoder.c#L26-L67

Hola Eliana, como estas? Te queria comentar que estuve intentando implementar este ultimo feature (he guardado lo que he hecho hasta ahora en "dev_not_working"). Sin embargo, estoy viendo que, por ejemplo si corro el programa para cifrar en Cesar, el programa me pisa el atributo "offset" de cesarEncoder_t (y lo mas raro, es que segun pude verificar, me lo piso al momento de leer el primer chunk de 64 bytes de stdin). Ademas, en todos los cifrados Valgrind me muestra errores, y en RC4 a veces muestra "Segmentation Fault" y a veces no, pero siempre falla. Mi pregunta es, o bien, si hay otra manera mas facil de simplificar ese codigo sin utilizar punteros a funcion, o por otro lado, como utilizarlos correctamente.

Elianagam commented 3 years ago

No usar buffers de longitud constante https://github.com/joaquinfontela/tp1-taller/blob/d73293bbfda64a3e6926602c4680a5b133f9bdb0/client_main.c#L29

Con esto te referis a que deberia hacer algo del estilo:

"

define BUFFER_LEN 70

(...) char buffer[BUFFER_LEN]; " ?

Con esto me refiero que si no utilizas exactamente 70 chars no deberias crear un buffer de esa longitud, el espacio sobrante puede contener datos basura que arruinen el comportamiento del programa (aunque pasen las pruebas)

Con respecto al camel case, podes dejarlo, pero es una sugerencia que no se mezclen snake con camel en todo el codigo, queda desprolijo y no da gusto leerlo Y ademas al utilizar nombre de funciones muy largas lo que realmente hace la funcion queda poco claro

Elianagam commented 3 years ago

No hagas esto, inicializalo sin definirlo https://github.com/joaquinfontela/tp1-taller/blob/d73293bbfda64a3e6926602c4680a5b133f9bdb0/client_socket.c#L7

Que deberia hacer aca? Ya que si unicamente lo declaro ("socket newSocket;") me lanza error. Pero no se como inicializar un struct sin definirlo.

Lo que falta es hacer el socket_init antes de asignarlo al client_socket -> socket Algo asi

socket_t s; init_socket(s) client->socket = s;

Elianagam commented 3 years ago

En vez de unsigned int podes usar los unit32_t, etc https://github.com/joaquinfontela/tp1-taller/blob/d73293bbfda64a3e6926602c4680a5b133f9bdb0/common_cesar.c#L24

Unisigned char (? https://github.com/joaquinfontela/tp1-taller/blob/d73293bbfda64a3e6926602c4680a5b133f9bdb0/common_cesar.c#L25

Aca ya he hecho los reemplazos de "unsigned int" por "uint_32", aunque no comprendo la ventaja. Por otro lado, que es lo que esta mal cuando declaro "current char" como "unsigned char"? Estoy extrayendo un caracter de la string.

El uni32_t unit8_t ya la estructura dice que es sin signo y sabes cuantos bytes estas utilizando de entrada. Lo mismo con los char, en vez de utilizar unsigned char se puede reemplazar por unit8_t. No es que este mal lo que haces, son sugerencias para mejorar la estructura del codigo y su lectura

Elianagam commented 3 years ago

Muy similar este codigo, busca la forma de achicarlo https://github.com/joaquinfontela/tp1-taller/blob/d73293bbfda64a3e6926602c4680a5b133f9bdb0/common_encoder.c#L26-L67

Hola Eliana, como estas? Te queria comentar que estuve intentando implementar este ultimo feature (he guardado lo que he hecho hasta ahora en "dev_not_working"). Sin embargo, estoy viendo que, por ejemplo si corro el programa para cifrar en Cesar, el programa me pisa el atributo "offset" de cesarEncoder_t (y lo mas raro, es que segun pude verificar, me lo piso al momento de leer el primer chunk de 64 bytes de stdin). Ademas, en todos los cifrados Valgrind me muestra errores, y en RC4 a veces muestra "Segmentation Fault" y a veces no, pero siempre falla. Mi pregunta es, o bien, si hay otra manera mas facil de simplificar ese codigo sin utilizar punteros a funcion, o por otro lado, como utilizarlos correctamente.

No te mates con esto y si o si trates de reducirlo a toda costa, si no se puede y lleva mucho tiempo no es algo que te haga desaprobar, pero tenelo en cuenta para proximos diseños

joaquinfontela commented 3 years ago

Hola Eliana, ahi realice todas las correcciones pedidas, a excepcion del problema del codigo duplicado, que si tengo tiempo intentare encontrarle la vuelta. Ya lo hice de dos maneras distintas y no me funciona, y quiero seguir avanzando con el TP2, pero si me queda tiempo, lo voy a revisar.

joaquinfontela commented 3 years ago

Ahi realice una entrega exitosa en el Sercom.

joaquinfontela commented 3 years ago

Me gustaria llegar al 7 por esa cuestion de que no se realizan los TPs nuevamente en caso de recursar, asi que si mi TP no esta para esa nota, agradezco si me podes avisar asi intento realizar alguna correccion mas.

Elianagam commented 3 years ago

Nota 9

joaquinfontela commented 3 years ago

Gracias!

joaquinfontela / Taller-95.08-TP1

Correcciones #1

define BUFFER_LEN 70

define BUFFER_LEN 70