EJ10TP11

[lucaMesi]

Hola buenas tardes, hice este ejercicio que me funciona pero no me gusta como me quedo. Les queria pedir si alguno lo puede ver y decirme que cosas podria mejorar o hacer mas sencillas. Muchas gracias. (Por ejemplo lo de la funcion neutro)

#include <stdio.h>
#include <strings.h>
#include "10.h"
#include <stdlib.h>

typedef struct tNode {
    elemType elem;
    int count; //cont de reps
    struct tNode * tail; //punt al aiguiente
}tNode;

typedef tNode* tlist;

typedef struct bagCDT {
    int size; //tamanio de la lista
    compareFn cmp; //guardamos la funcion compare para usarla
    tlist first;
} bagCDT;

typedef void (*operator)(int * count, int * size);

static void suma(int * count, int * size){
    (*count)++;
}

static void resta(int * count, int * size){
    (*count)--;
    if(*count == 0){
        (*size)--;
    }
}

static void neutro(int * count, int * size){
    return;
}

static void freeNode(tlist list){
    if (list == NULL){
        return;
    }
    freeNode(list->tail);
    free(list);
}

/* Libera toda la memoria reservada por el TAD */
void freeBag( bagADT bag){
    freeNode(bag->first);
    free(bag);
}

/* Retorna un nuevo bag de elementos genéricos. Al inicio está vacío */
bagADT newBag(compareFn cmp){
    bagADT aux = malloc(sizeof(bagCDT));
    aux->size = 0;
    aux->first = NULL;
    aux->cmp = cmp;
    return aux;
}

//busca en la lista cuantas veces aparece un numero
static unsigned int search_change_count(tlist list, elemType elem, bagADT bag, operator apply){
    if (list == NULL){
        return 0;
    }

    if (bag->cmp(list->elem, elem) == 0){
        apply(&list->count, &bag->size);
        return list->count;
    }
    return search_change_count(list->tail, elem, bag, apply);
}

/* Inserta un elemento. Retorna cuántas veces está
** elem en el conjunto luego de haberlo insertado (p.e. si es la 
** primera inserción retorna 1).
*/
unsigned int add(bagADT bag, elemType elem){
    //si count == 0 agrego un nuevo nodo al principio de la lista
    if(count(bag, elem) == 0){
        tlist aux = malloc(sizeof(tNode));
        aux->elem = elem;
        aux->count = 1;
        aux->tail = bag->first;
        bag->first = aux;
        bag->size++;
        return aux->count;
    }
    return search_change_count(bag->first, elem, bag, suma);
}

/* Retorna cuántas veces aparece el elemento en el bag */
unsigned int count(const bagADT bag, elemType elem){
    return search_change_count(bag->first, elem, bag, neutro);
}

/* Retorna la cantidad de elementos distintos que hay en el bag */
unsigned int size(const bagADT bag){
    return bag->size;
}

/* Remueve una aparición de un elemento. Retorna cuántas veces está
** elem en el conjunto luego de haberlo borrado 
*/
unsigned int delete(bagADT bag, elemType elem){
    if (count(bag, elem) > 0){
        return search_change_count(bag->first, elem, bag, resta);
    }
    return 0;
}

static elemType higher_count(tlist list, compareFn cmp, bagADT bag){
    if(list == NULL){
        return NULL; 
    }

    elemType aux = higher_count(list->tail, bag->cmp, bag);
    if (aux == NULL){
        return list->elem;
    }

    if (list->count >= count(bag, aux)){
        return list->elem;
    }
    return aux;
}

elemType mostPopular(bagADT bag){
    if(bag->first == NULL){
        exit(EXIT_FAILURE); //por si es vacio
    }
    return higher_count(bag->first, bag->cmp, bag);
}

[marcelogarberoglio]

Hay cosas para mejorar. Creo que por el intento de hacer funciones genéricas te complicaste un poco, por ejemplo en la función search_change_count cuando le pasás la función resta, apenas lo encuentra le resta uno, incluso si la cantidad es cero, quedando con cantidad negativa En higher_count preguntás si retorna NULL, pero eso es válido únicamente si elemType es un puntero. mostPopular sería más sencillo si la hacés iterativa Y si decidiste guardar los elementos en una lista (que está bien) podrías aprovechar para guardarlos ordenados, de esa forma podrías evitar recorrer hasta el final si el elemento no está.

[lucaMesi]

Claro yo pense lo mismo, estaba haciendo muchas funciones de mas que y se me estaba haciendo mucho quilombo. Tome esa decision de resolver en funciones auxiliares porque supuse que la mejor idea era recorrer la lista recursivamente pero evidentemente me complique mucho, pero no se me ocurria sino como hacerlo. Lo de que cuando le paso la funcion resta, que apenas lo encuentra le resta uno, no me lo salva el if que puse en la funcion "delete"? Gracias por las correciones.

[marcelogarberoglio]

Lo de la función resta sí te lo salva delete, pero está recorriendo dos veces. Además justamente esa función depende de lo que hagan otras. Si cambiás delete para no recorrer dos veces, tenés que ver si ese cambio afecta a otra función, por ejemplo resta. Eso muestra que el código está bastante acoplado, lo cual lo hace complicado de seguir y muy complicado de mantener.

[lucaMesi]

A okey, lo tendre en cuenta para la proxima entonces. Muchas gracias