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

feature #19 #20

Merged
merged 5 commits into from
Dec 9, 2018
Merged

feature #19 #20

merged 5 commits into from
Dec 9, 2018

Conversation

DTONeill
Copy link
Contributor

@DTONeill DTONeill commented Dec 3, 2018

Translator class could be added to the project to translate keys directly.

Copy link
Member

@mdeslauriers-sigmund mdeslauriers-sigmund left a comment

Choose a reason for hiding this comment

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

Il semble manquer les fichiers suivant dans les commits :

[HttpGet]
public HttpResponseMessage Translate(string text, string translate_from, string translate_to)
{
var translator = DictionaryKeyTranslatorProvider.GetTranslator();

Choose a reason for hiding this comment

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

Devrait avoir une validation de TranslationAvailable() avant d’appeler le translator pour éviter les null references.
Retourner une réponse HTTP 400 BadRequest indiquant qu'aucun translator n'est configuré.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

README.md Outdated
## Bouton de traduction automatique

La traduction automatique vous permet d'implémenter une classe qui sera utilisée pour traduire les textes des clés.
Pour ce faire, définissez une classe implémentant l'interface "Translator" et configurez l'application au démarrage pour en faire utilisation.

Choose a reason for hiding this comment

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

Par convention, pour une interface on devrait utiliser ITranslator
Je proposerait peut-être un terme plus spécifique du genre IDictionaryKeyTranslator

Choose a reason for hiding this comment

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

En y repensant, je me demande si on ne devrait pas plutôt référer à "DictionaryValueTranslatorProvider" étant donné que c'est la valeur qu'on traduit et non la clé elle même. partout où on réfère à DictionaryKey il faudrait s'assurer que c'est bien de la clé qu'on parle et non de la valeur. On devrait utiliser DictionaryValue lorsqu'il s'agit de la valeur textuelle associée à la clé, pour une langue spécifique.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@DTONeill DTONeill merged commit 74bc97e into master Dec 9, 2018
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.

2 participants