Skip to content
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

New feature config_remove_key #96

Merged
merged 2 commits into from
Aug 26, 2018

Conversation

FernandoVelcic
Copy link
Contributor

No description provided.

@gastonprieto
Copy link
Contributor

Mi duda sobre el remove es si queremos que no falle si no existe la key.

@FernandoVelcic
Copy link
Contributor Author

@gastonprieto si puede ser, siguiendo la lógica de otras partes de la biblioteca como por ejemplo dictionary_remove, no explota si el elemento no existe, pero la función retorna un puntero al elemento removido o devuelve NULL si no existe.

En el caso de config_remove_key entiendo que no sería comodo o útil retornar un puntero al elemento removido porque el programador debería encargase de la interpretación de ese puntero, si es un long, double, int, array, string, etc y de su liberación en la memoria. Lo que si me parece bueno y puedo agregar es que la función config_remove_key retorne un booleano para saber si se removio alguna clave.

@gastonprieto
Copy link
Contributor

@gastonprieto si puede ser, siguiendo la lógica de otras partes de la biblioteca como por ejemplo dictionary_remove, no explota si el elemento no existe, pero la función retorna un puntero al elemento removido o devuelve NULL si no existe.

Igual es medio inconsistente como lo tenemos, porque en dictionary es un error silencioso, pero en las listas asume que siempre existe, por lo tanto pincharía. Pero nada, dado que esto se parece mucho a un dictionary mantendría la consistencia entre estos dos tipos.

En el caso de config_remove_key entiendo que no sería comodo o útil retornar un puntero al elemento removido porque el programador debería encargase de la interpretación de ese puntero, si es un long, double, int, array, string, etc y de su liberación en la memoria. Lo que si me parece bueno y puedo agregar es que la función config_remove_key retorne un booleano para saber si se removio alguna clave.

Por ahora dejemoslo así. Igual ojo que lo que tiene siempre internamente ese config son Strings, por lo tanto si devolvieramos el valor removido siempre sería un String.

@gastonprieto
Copy link
Contributor

En resumen 👍

@FernandoVelcic
Copy link
Contributor Author

Igual ojo que lo que tiene siempre internamente ese config son Strings, por lo tanto si devolvieramos el valor removido siempre sería un String.

Si, no me exprese bien, me refería a la interpretación del dato dentro de ese string por si al programador le interesa usarlo, pero es verdad que el string tambien le puede ser útil, por ejemplo para hacer un nuevo config_set_value.

Copy link
Contributor

@tferraro tferraro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lo mismo que comentó @gastonprieto sobre que no falle ni nada, pero es verdad que viene con la misma idea que el diccionario.

Tomando en cuenta que uno llamaría a la función para eliminar algo, por reciprocidad, sabemos que ese algo existe. En caso contrario, si llamamos la función, podemos esperar varios comportamientos según como sea la biblioteca.

TL;DR: 👍

@gastonprieto gastonprieto merged commit 1624559 into sisoputnfrba:master Aug 26, 2018
@gastonprieto
Copy link
Contributor

@FernandoVelcic Gracias, mergeado!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants