marcosmarp / dolar-bot

Reddit bot for answering 'dolar' mentions with the official, underground and the rest of the multiple cotizations of it in AR$ pesos.
MIT License
5 stars 2 forks source link

Update functions.py #1

Closed agustiza closed 2 years ago

agustiza commented 2 years ago

Muy buena la idea del bot!

Si no molesta la sugerencia, los ifs tan nesteados son dificiles de leer y en general son mala práctica. Por lo general un early return lo hace mas legible. El código es funcionalmente el mismo y traté de tocar lo menos posible.

Otra sugerencia es que es conveniente dejar globales o configuración afuera del código cosa de minimizar cambios a futuro (Single Responsability Principle). Por ejemplo en la versión anterior agregar un subreddit nuevo es tener que trackear varias funciones y si te olvidas una falla.

Idealmente se manejan o en variables de entorno o archivo de configuracion pero medio overkill en este caso. Sí sacaría la lista subreddits y la pasaría a main (por ej llamar a init_bot con parametros una vez) pero ya se está haciendo tarde.

Cualquier cosa avisame. Saludos!

PD: No tenía un ide a mano así que es posible que haya algún que otro error de sintaxis.

marcosmarp commented 2 years ago

Hola Agustín! Gracias por las recomendaciones y las voy a aplicar!

Solo hay un temita, parece que cuando hiciste la PR había dos commits que tu fork no tenía aplicadas, entonces hay una contradicciones entre está PR y main

Que opinas que debemos hacer? Un rebase?

agustiza commented 2 years ago

Vi que pusheaste un par de cosas. Como sacar a los de republica argentina jaja. El dolar blue va en contra del relato?

Si te parece mergeo contra lo último de upstream a manopla y de paso le paso lo de los subs a main cosa de poder configurarlo más fácil.

marcosmarp commented 2 years ago

Jajajaj uno de los locos me dijo que al pedo tener en el sub un bot de dolar porque el dolar """"está estable"""" y el bot sería spam. Mamita

Agregale nomás a manopla y mergeamos tu PR

no-username-123 commented 2 years ago

No te convendria usar subreddit.stream.comments() para conseguir los comentarios? Encima podes hacer uso de multireddit para conseguir los comentarios mas recientes de varios subreddits. Ej:

subreddits = ['wallstreetbets', 'askreddit']
for comment in reddit.subreddit('+'.join(subreddits)).stream.comments():
    print(comment.subreddit, comment.body)
agustiza commented 2 years ago

No te convendria usar subreddit.stream.comments() para conseguir los comentarios? Encima podes hacer uso de multireddit para conseguir los comentarios mas recientes de varios subreddits. Ej:

subreddits = ['wallstreetbets', 'askreddit']
for comment in reddit.subreddit('+'.join(subreddits)).stream.comments():
    print(comment.subreddit, comment.body)

Si, totalmente! Pasaría de un poll a un stream y cambia sustancialmente el funcionamiento del bot (simplificandolo bastante).

Preferí limitarme a legibildiad y estilo mas que funcionamiento en sí.

Termino de mergear y hago uno con stream o si te copás mandale vos.

Jajajaj uno de los locos me dijo que al pedo tener en el sub un bot de dolar porque el dolar """"está estable"""" y el bot sería spam. Mamita

Como quema la cabeza la droga polenta

marcosmarp commented 2 years ago

No te convendria usar subreddit.stream.comments() para conseguir los comentarios? Encima podes hacer uso de multireddit para conseguir los comentarios mas recientes de varios subreddits. Ej:

subreddits = ['wallstreetbets', 'askreddit']
for comment in reddit.subreddit('+'.join(subreddits)).stream.comments():
    print(comment.subreddit, comment.body)

Muy buena esa, no la conocía. Ahora cuando mergeo lo de Agustín lo hago

marcosmarp commented 2 years ago

Resuelto en último commit