2

Estoy intentando ordenar alfabéticamente por método Shell un arreglo de estructuras, solamente contiene Nombre, Apellido, Edad e Indice, algo muy básico.

Ahora bien, el método solo ordena cuando el vector es menor a 4, de otra forma no muestra el vector ordenado, aquí dejo el código:

struct Estudiante{
    char apellido[20];
    char nombre[20];
    int edad;
    int indice;
};

Y el método shell:

void ordenarEstudiantes(Estudiante *e, int tam){
    int j, k, salto;
    Estudiante aux;

    salto = tam / 2;

    while(salto > 0){
        for(int i = salto; i < tam; i++){
            j = i - salto;

            while(j >= 0){
                k = j + salto;

                if(strcmp(e[j].apellido, e[k].apellido) < 0){
                    j = -1;
                }else{
                    aux = e[j];
                    e[k] = e[j];
                    e[k] = aux;
                }
            }           
        }
        salto = salto / 2;
    }
}

Gracias!

2 respuestas 2

2

Así a bote pronto el error más destacable de tu código es el siguiente:

}else{
  aux = e[j];
  e[k] = e[j];
  e[k] = aux;
}

Ahí estás intentando hacer un swap de e[j] y e[k], es decir, intercambiar sus valores... pero lo estás haciendo mal. Fíjate que e[k] lo asignas dos veces seguidas... no tiene buena pinta.

}else{
  aux = e[j];
  e[j] = e[k];
  e[k] = aux;
}

Y luego, a modo de consejo, intenta que las variables tengan la menor vida posible... En tu caso concreto, j, k y aux tienen una vida excesivamente larga. Acortar la vida de las variables al mínimo imprescindible ayuda a evitar errores bastante complicados de encontrar.

void ordenarEstudiantes(Estudiante *e, int tam){
int salto = tam / 2;

while(salto > 0){
    for(int i = salto; i < tam; i++){
        int j = i - salto;

        while(j >= 0){
            int k = j + salto;

            if(strcmp(e[j].apellido, e[k].apellido) < 0){
                j = -1;
            }else{
                Estudiante aux = e[j];
                e[j] = e[k];
                e[k] = aux;
            }
        }           
    }
    salto /= 2;
}

EDITO: A instancias de @PaperBirdMaster para mejorar el código

Una alternativa más limpia para intercambiar el valor de dos variables es usar la función swap:

while(salto > 0){
    for(int i = salto; i < tam; i++){
        int j = i - salto;

        while(j >= 0){
            int k = j + salto;

            if(strcmp(e[j].apellido, e[k].apellido) < 0){
                j = -1;
            }else{
                swap(e[j],e[k]);
            }
        }           
    }
    salto /= 2;
}

Otra posible mejora a implementar en tu código es el uso de la clase string en vez de recurrir a char*. Los strings a la usanza de C presentan muchas deficiencias en C++:

  • Hay que estar pendientes de su tamaño todo el rato
  • Si se crean en la pila no se puede modificar su tamaño máximo
  • Hay que usar funciones externas para modificar su contenido
  • Posibilidad de acceder a memoria no válida
  • Posibilidad de tener fugas de memoria

La clase string se diseñó con la idea de evitar que el programador tuviese que enfrentarse a todos estos problemas constantemente. Además la sobrecarga de operadores que incorpora hace que su uso sea bastante natural:

struct Estudiante{
    string apellido;
    string nombre;
    int edad;
    int indice;
};

Estudiante e;
e.nombre = "Juan";
e.apellido = "Perez";

Un saludo.

7
  • Muy bien visto eferion; ¿puedo sugerirte que comentes el uso de std::swap como alternativa al swap manual y la substitución de arrays estáticos de char por std::string? Lo haría yo mismo pero no creo que pueda aportar nada interesante que no hayas aportado ya. el 23 may. 2016 a las 8:38
  • Claro, lo edito en seguida. Pensé en ponerlo pero no quería alejarme mucho del código inicial porque tiene toda la pinta de ser un ejercicio de clase y éstos suelen estar limitados en cuanto a llamadas a la STL :)
    – eferion
    el 23 may. 2016 a las 8:40
  • sólo tenemos nuestra intuición para determinar que es un ejercicio de clase pues (salvo error u omisión) no me parece que Jesús V. haya comentado que lo sea. Por otro lado, aún siendo un ejercicio académico, conocer más alternativas no hace daño, al contrario: amplía los horizontes del estudiante. el 23 may. 2016 a las 8:43
  • Por ese motivo te he hecho caso y he ampliado la respuesta. Con tu comentario me ayudaste a despejar las dudas sobre si recargar la respuesta o no
    – eferion
    el 23 may. 2016 a las 8:51
  • Aunque sin el añadido la respuesta era ya muy buena, ahora ha quedado una respuesta mejor :) ¡buen trabajo! el 23 may. 2016 a las 9:31
0

Lo mejor, cuando la opción existe, es utilizar una buena librería que haga el trabajo por ti.
En este caso ni siquiera hace falta una externa. C++ viene de casa, en versiones modernas, con std::sort en <algorithm>.

#include<algorithm>
#include<string.h>
#include<iostream>

struct Estudiante{
    char apellido[20];
    char nombre[20];
    int edad;
    int indice;

    bool operator < (const Estudiante& e) const
    {
        return strcmp( apellido, e.apellido ) < 0;
    }
};


void ordenarEstudiantes(Estudiante *e, int tam){
  std::sort( e, e+tam );
}

int main(int argc, char** argv) {
    Estudiante arregloEjemplo[] = {
        { "Perez", "Juan", 2, 1 },
        { "Lopez", "Super", 23, 2 },
        { "Thatcher", "Margaret", 87, 3},
        { "Pi", "Filemon", 48, 4}
    };
    ordenarEstudiantes( arregloEjemplo,
                        sizeof(arregloEjemplo)/sizeof(*arregloEjemplo) );
    for ( const auto& e : arregloEjemplo )
    {
        std::cout << e.apellido << "," << e.nombre << "," << e.edad <<
                     "," << e.indice << std::endl;
    }

    return 0;
}

Tu Respuesta

Al pulsar en “Publica tu respuesta”, muestras tu consentimiento a nuestros términos de servicio, política de privacidad y política de cookies

¿No es la respuesta que buscas? Examina otras preguntas con la etiqueta o formula tu propia pregunta.