Longer response:

NB I've not used the system and only quickly reviewed
https://py-googletrans.readthedocs.io/_/downloads/en/documentation/pdf/

NBB I am treating you (and/or other interested-readers) as something of
a 'beginner'. No insult is intended should I appear to be 'talking down'.



On 18/04/2021 10.56, Quentin Bock wrote:
> I'm trying to take the user input and let them change the target language
> or dest
> code:
> 
> from deep_translator import GoogleTranslator
> import googletrans
> import sys
> 
> language_list = googletrans.LANGUAGES
> print(language_list)


Why print this? Should this output be labelled, or otherwise explained
to the user?

Looking at what is output (yourself), is it a list or a dictionary? When
using it, is the code referring to a list or list of dictionary keys
(country-codes), should it be looking at the country-names, or even both?
(I'm not sure of your intent)


> feedback = input("Would you like to translate a sentence of your own?
> (yes/no)")
> 
> if feedback == 'no':
>     sys.exit()

When reviewing code: if the same lines are (basically) repeated, is this
(often) a recommendation to employ a function?

Given that the user has kicked-off the program[me], is it necessary to
ask if (s)he wants to translate some text? Will anyone reply "no" at
this point?

Thus, (instead of asking) could we reasonably set "feedback" to "yes"?

That said, what if the user replies "YES" (or some less logical
combination of the same letters)?


> while feedback == 'yes':

Some might suggest that "feedback" should be a boolean value, thus:

while feedback:


>     user_choice = input ("Enter a language (the abbreviation or correctly
> spelled name of the language): ").lower()

So, the acceptable response is either the code or the name of the language?


>     user_sentence = input("Enter a sentence you want translated: ")
> 
>     user_translation = GoogleTranslator(source='en',
> target=user_choice).translate(user_sentence)

Yet (according to the docs), the "target" value should be a
language-code, eg 'fr' rather than 'french'.
(remember: I've not used the library, so you're the expert...)


>     print(user_translation)

Some would suggest that if "user_translation" is defined on one line and
only ever used on the following, the two lines should be conflated.

(some of us don't agree, especially if it makes testing more awkward)


>     feedback = input ("Would you like to translate a sentence of your own?
> (yes/no)")
> 
>     if feedback == 'no':
>         sys.exit()

A UX (User Experience) consideration:
If the user has a series of translations to be made, will (s)he want to
keep repeating the same language/code? Could you code the input
statement to have a default value on the second and ensuing times
through the loop? In this mode, if the user hits Enter without entering
any text-data, the code could 'remember' the language and save
manual-effort.


A thought about design:
This code consists of a loop which has two purposes
1 translate some text
2 continue looping or stop

A useful guide (and testing aid) is "SRP" (the Single Responsibility
Principle) which has a more-technical definition, but let's stick to its
name. Each routine should have only a single task to perform (and should
do it well). Let's consider the first 'purpose':

def translate():
    user_choice = input ("Enter...): ").lower()
    user_sentence = input("Enter a sentence you want translated: ")
    user_translation = GoogleTranslator(source='en',
target=user_choice).translate(user_sentence)

Applying SRP again, we might debate that last line, because it is not
(easily) possible to distinguish between an error at class
instantiation-time (eg "target" is not an acceptable language-code), and
an error at translation-time.

Let's go the next step and separate-out the input functionality. How
about a function like:

def translate( target_language_code, english_text ):
    translator = GoogleTranslator( source='en',
                                   target=target_language_code
                                 )
    return translator.translate( english_text )

Why?

There is virtue in what appears to be extra, even unnecessary, coding
effort!

This is very easily tested - and can be (even more easily) automatically
tested:

def test_translator( ... ):
    assert translate( "fr", "hello" ) == "bonjour"
    assert translate( "de", "thank you" ) == "danke schön"
    ... test for error using non-existent language-code
    ... test for error using non-English text


I'll leave you to work-out how to encapsulate the inputs into a function
- especially if you plan to implement the default-value idea/service to
users. (!)


The second purpose, after extracting the above, leaves us with a
"control loop" (in real-time systems it is termed an "event loop"), and
pretty-much the code as-is (after any amendments per comments-above).

while feedback:
    print( translate() )
    feedback = input ("Would you like to translate a sentence of your own?
(yes/no)")
    if feedback == 'no':
        sys.exit()

That's the second purpose done!
- or is it?


Consider the while - it creates an "infinite loop". If we ever write
code like this, the *second* thought should *always* be: "how am I going
to stop it?"!

You did - well done!


However, let's ask: what happens when the loop ends? Answer: the program
stops. Duh!

So, why do we have a sys.exit() inside the loop? Couldn't that be taken
for granted if we re-word the use of "feedback" appropriately, to
control the repetition?
(which follows from earlier comment on the same identifier!)

Within that, here's another UX/design consideration: are we only
interested in "yes" and "no" - what about all the other things a user
might type, mis-type, or other mistakes which could be made? Perhaps we
should consider a convention (or would it become a "rule"?) that if the
user types *any* text beginning with "y" (or "Y"?), then the loop will
repeat - whereas any other text (or nothing but the enter key) will be
considered as 'no'. Perhaps you'd prefer to be more positive, er,
negative, in this choice? Regardless, we can refine it down to something
like:

translations_to_do = True
while translations_to_do:
    print( translate() )
    feedback = input (...)
    translations_to_do = feedback and feedback[ 0 ] in [ 'n', "N" ]

Why the two halves to the condition? Consider the case where the use
types nothing (except the enter key): what would happen when the code
"feedback[ 0 ]" was executed?


Once again, refining SRP to the nth-degree, perhaps we could take the
two 'control lines' out of the loop and replace them with a function call:

continue_looping = True
while continue_looping:
    print( translate() )
    continue_looping = fetch_user_feedback()

Does that look like "clean code"? Is it minimalist, yet also clearly
communicating what the program will do?


The criticism of such an approach is that you've ended-up with a bunch
of small functions. Thus, the logic may appear fractured or the
code-body feel 'bitsy'.

The former shouldn't become the case (IMHO) and is largely countered by
carefully-considered choices for [function] names. These in-turn
[should] make the code eminently 'readable'. The second is true, *but*
each small routine can be tested independently of [all] the others.
There's plenty of room to discuss "dependence" and "independence"! For
the purposes of our code-review: each routine can be changed internally
(ie providing its "signature" remains the same) without affecting the
total, ie not requiring any code to change in some other routine (that
the "independence" bit!). Plus, each routine can be easily "re-used", ie
is virtuous according to Object-Oriented philosophy.


A worthwhile separation is to keep 'I/O' distinct from 'process'. Let's
go a little crazy to illustrate that ideal:-

Imagine that someone likes this program, but wants a whole series of
text-items translated. His text is stored in a database table (and he
doesn't want to manually re-type 'everything', despite thinking your
program is 'the best thing since sliced bread").  What now?

A second program, with almost identical code? What about that SRP hassle
then?

Well, we can very easily pick-and-choose from amongst all those small
functions to "re-use" some as-is, and replace the ones handling manual
input with database queries. Hey, a great win!


However, *my* demo above shows me as not following 'the rules'. What?
Shock, horror!

That's the problem with "print( translate() )" - and we're back to an
earlier comment - perhaps it should be:

    translation = translate()
    report( translation )

Now, the report() function can be responsible for deciding if the
translation should be printed (original spec) or INPUT to a database table.


We've well-and-truly disappearing down a 'rabbit hole' now! (Apologies,
- I noted Chris making "Alice in Wonderland" references) If you haven't
already, now is a good time to ask just how relevant some of these
'rules' - or are they 'ideas' - might be!

Here's another: "YAGNI" (You Aren't Going to Need It) - don't try to
make a simple program into the be-all-and-end-all just in-case some
idiot (I mean: nice person, like me) comes along asking for
database-output. That said, designing for 'independence' will facilitate
such extensions, should they (ever) be required.

- and another: "YMMV" (Your Mileage May Vary) - from the motor industry
expecting us to excuse their outlandish claims about how little fuel a
car requires/how many miles or km it will travel on a single
electric-charge. We apply it to say that what my team thinks is 'the one
true way' to do things, may be quite different to what you/your team
think is 'correct'!


NB 'umble scribe has (lazily) not tested these code-snippets


Web.Ref:
https://towardsdatascience.com/5-principles-to-write-solid-code-examples-in-python-9062272e6bdc
-- 
Regards,
=dn
-- 
https://mail.python.org/mailman/listinfo/python-list

Reply via email to