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

Messaggi di benvenuto più articolati #42

Closed
wants to merge 4 commits into from
Closed

Messaggi di benvenuto più articolati #42

wants to merge 4 commits into from

Conversation

Picred
Copy link
Member

@Picred Picred commented Oct 21, 2023

Refactor:

  • ho spostato le funzioni e file di dati usati
  • ho usato Updater piuttosto che ApplicationBuilder per il bot

Feat:

  • Aggiunto link al topic readme.
  • Aggiunta una frase incoraggiante presa randomicamente.

I test dovrebbero essere riscritti (chiaramente) e si dovrebbe testare la funzionalità del bot nei casi "più estremi" (io l'ho testato semplicemente con un nuovo user in un gruppo)

if not new_member['is_bot']:
update.message.reply_text(f'{generate_welcome(new_member)}')
if update.message.new_chat_members:
for new_member in update.message.new_chat_members:
Copy link
Member

Choose a reason for hiding this comment

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

Il controllo dell'esistenza di elementi diventa ridondante, infatti un foreach eventuale su una lista vuota non avrebbe tempo di calcolo aggiuntivo

Copy link
Member Author

Choose a reason for hiding this comment

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

Scusami ma non ho capito benissimo. La miglior scelta è non eseguire il for?

Copy link
Member

@domenicoblanco domenicoblanco Oct 24, 2023

Choose a reason for hiding this comment

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

In questo caso conviene eseguirlo a prescindere, infatti il costo del for su una lista vuota è praticamente identico al controllare se esiste effettivamente qualcosa su cui iterare ed eventualmente iterare.
In breve, invece di controllare due volte l'esistenza degli elementi, conviene lasciarlo controllare dal for stesso

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah ceeerto, ha perfettamente senso. Sono d'accordo


def handle_welcome(update: Update, new_member: User) -> None:
welcome_msg = f'{generate_welcome(new_member)}\n' + \
f"- Dai un'occhiata al [README]({welcome['readme']})\n" + \
Copy link
Member

Choose a reason for hiding this comment

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

L'uso di ' e " non è consistente con il resto del programma, inoltre la concatenazione attraverso l'operatore + rende inutile l'uso delle f-string.
Una sintassi valida potrebbe essere simile a:

welcome_msg = (
    f'{generate_welcome(new_member)}\n' 
    f'- Dai un'occhiata al [README]({welcome["readme"]})\n'
    f'- {welcome["utils"][randrange(0, len(welcome["utils"]))]}'
)

"en": ["Welcome USER to our group ^-^","Hello USER!","Howdy USER!"]
"en": ["Welcome USER to our group ^-^","Hello USER!","Howdy USER!"],
"readme": "https://t.me/c/1095167198/67194",
"utils": [
Copy link
Member

Choose a reason for hiding this comment

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

Al fine di mantenere il supporto alla doppia lingua (e ulteriori future) penso possa essere utile trasformare le liste contenute in it ed en in dizionari e incorporare all'interno di questi nuovi dizionari utils in entrambe le lingue

def handle_welcome(update: Update, new_member: User) -> None:
welcome_msg = f'{generate_welcome(new_member)}\n' + \
f"- Dai un'occhiata al [README]({welcome['readme']})\n" + \
f"- {welcome['utils'][randrange(0, len(welcome['utils']))]}"
Copy link
Member

Choose a reason for hiding this comment

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

Facendo riferimento al commento su utils, andrebbero localizzate le stringhe, eventualmente memorizzando il valore per l'uso successivo

Copy link
Member Author

@Picred Picred Oct 24, 2023

Choose a reason for hiding this comment

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

E come potrebbe diventare "randomico" il messaggio se appena ne leggo uno casuale dal file (la prima volta), esso viene localizzato e ripreso in una futura chiamata della funzione?

Copy link
Member

Choose a reason for hiding this comment

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

Errore mio, intendevo memorizzare la lingua di riferimento in modo da essere passata all'interno di generate_welcome, senza dover quindi ottenere di nuovo il valore da User

Copy link
Member Author

@Picred Picred Oct 24, 2023

Choose a reason for hiding this comment

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

Quindi l'idea sarebbe creare una sorta di:

{
     "it" : {
       "benvenuti" : ["...", "..." ],
       "utils" : ["...", "...", "..." ]
      }
      "readme" : "https://..."
}

Ho capito bene?

Ed eventualmente il lan_code conservato in una variabile per accedere direttamente al json[lan_code]

Copy link
Member

Choose a reason for hiding this comment

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

Esattamente c:

@Picred
Copy link
Member Author

Picred commented Oct 24, 2023

#44

@Picred Picred closed this Oct 24, 2023
@Picred Picred deleted the new-welcomes branch October 25, 2023 14:40
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