El problema, como te han comentado, se encuentra en el último bucle de ordenarArreglo
:
for(int z = 0;z <= 10;z++)
{
cout << " " << arr[z];
}
En ese bucle estás iterando en el rango (0,10), cuando el rango válido es (0,9).
La solución más sencilla pasa por cambiar <=
por <
:
for(int z = 0;z < 10;z++)
{
cout << " " << arr[z];
}
Sin embargo, el valor 10
se repite por doquier. Un buen hábito consiste en evitar que este tipo de valores se repliquen por el código como conejos. Imagínate que para hacer pruebas prefieres usar un valor bajo (por ejemplo 10) pero que luego en el entorno real tienes que usar un valor más grande (supone 500). ¿Vas a localizar todas las apariciones para reemplazarlas una a una? Espero que tu respuesta no sea afirmativa.
La solución a este problema pasa por usar variables constantes. Esta forma de trabajar proporciona dos grandes ventajas:
- Es muy sencillo cambiar el valor. Lo editas en un sitio y todo el código se adapta al nuevo valor.
- Los nombres suelen ser más descriptivos que los números. A la hora de encontrar errores no es lo mismo encontrarse con un literal e intentar deducir si dicho valor es correcto que encontrarse con una constante y, a partir de su nombre, saber si estás usando la constante correcta.
Ejemplo:
const int TamVector= 10;
int ordenarArreglo(int *arr)
{
int i, j, temp = 0;
for(i = 0; i < TamVector - 1; i++) // <<--- AQUI!!!
{
for(j = 0; j<(TamVector - i - 1); j++) // <<--- AQUI!!!
{
// ...
}
}
// ...
for(int z = 0;z < TamVector;z++) // <<--- AQUI!!!
{
cout << " " << arr[z];
}
}
int main()
{
//Arreglo de diez numeros a modificar.
int arr[TamVector] = {15,1,3,4,8,9,7,6,7,4}; // <<--- AQUI!!!
cout << "El arreglo original es: ";
for(int i=0;i < TamVector; i++)
{
cout << " " << arr[i];
}
// ...
}
¿Y no sería mejor usar #define
?
Lo cierto es que no ya que #define
no dispone de un tipado fuerte, lo que puede provocar si no se tiene en cuenta ese pequeño pero crucial detalle.
Otro detalle tonto de tu código es la inicialización de temp
, i
, y j
en mitad de ordenarArreglo
. Ese código directamente sobra ya que esas variables no tienen uso. Aprovecho este párrafo para decirte también que esas variables deberías declararlas dentro del bucle correspondiente (como ya haces en otras secciones del código):
for(int i = 0; i < TamVector - 1; i++)
{
for(int j = 0; j<(TamVector - i - 1); j++)
{
if(arr[j] > arr[j + 1])
{
int temp = arr[j];
arr[j] = arr[j+1];
arr[j + 1 ]= temp;
}
}
}
Es altamente recomendable reducir el ámbito de las variables al mínimo para evitar errores tontos al programar. Crear una variable int
en cada iteración del bucle no tiene coste y, además, los compiladores pueden hacer maravillas al optimizar el código... mientras que alargar innecesariamente la vida de una variable puede dar lugar a problemas bastante complicados de encontrar ¿qué sucede si reutilizas una variable sin darte cuenta y asumes que está inicializada a un valor que no es el que realmente posee?
Otro detalle lo encontramos aquí:
temp = arr[j];
arr[j] = arr[j+1];
arr[j + 1 ]= temp;
Que se puede sustituir tranquilamente por esto:
std::swap(arr[j],arr[j+1]);
Otro problema que tiene tu código lo encontramos aquí:
int ordenarArreglo(int *arr)
{
// ...
// ¿Y el return?
}
Si indicas que la función devuelve un entero necesariamente deberías poner un return
en algún lado (preferiblemente al finalizar la función), sin embargo dicha instrucción brilla por su ausencia.
Esto genera un problema añadido y es que permite que la siguiente instrucción sea válida:
cout << ordenarArreglo(arr);
¿Realmente quieres imprimir un entero adicional después de mostrar el vector resultante por la pantalla? Imagino que no.
Resumiendo, la función debería tener la siguiente firma:
void ordenarArreglo(int *arr)
Y en la penúltima instrucción de tu programa sobra el cout
:
ordenarArreglo(arr);
return 0; // También puedes poner return EXIT_SUCCESS; que queda más elegante