Sending a message to a Telegram bot

jenya7

@JonB said in Sending a message to a Telegram bot:

@jenya7 said in Sending a message to a Telegram bot:

It stops in qstring.h at
inline bool QString::isEmpty()
const { return d->size == 0; }

In addition to @jsulm's excellent points. We don't know the scope of your sys_params (probably "global", yuck, don't do that), and:
typedef struct
{
   //some fields
    QString bot_api_key;
}SYS_PARAMS;
SYS_PARAMS sys_params;
We don't know whether you have written anything to the "some fields", or written directly into sys_params.

If I comment out the constructor stuff - I see all fields of sys_params perfectly initialized. The problem - it gets to
bot = new TelegramBot (sys_params.bot_api_key); before it's initialized. The constructor goes first and then goes sys_params initialization (SetupRun(); in main.cpp).

Why do I get to the constructor? I don't instantiate any telebot object at all in the code.
When I get to the constructor I see
sys_params.bot_api_key = <not accessible>
how can it be?

JonB

@jenya7 said in Sending a message to a Telegram bot:

I don't instantiate any telebot object at all in the code.

You do if you hit the constructor! And putting a breakpoint there would should you where you do so from.

jenya7

@JonB said in Sending a message to a Telegram bot:

@jenya7 said in Sending a message to a Telegram bot:

I don't instantiate any telebot object at all in the code.

You do if you hit the constructor! And putting a breakpoint there would should you where you do so from.

I see...
In telebot.h I have extern telebot m_telebot;
and in telebot.cpp telebot m_telebot;
in order to work with the object in other parts of the project.
And when I include telebot.h in main.cpp it gets initialized before sys_params is accessible.

JonB

@jenya7 said in Sending a message to a Telegram bot:

and in telebot.cpp telebot m_telebot;

Indeed, and that is where it will get instantiated.

Since you say it is m_telebot, is it a member variable in some class? Or is it a (mis-named) global? You will not want it to be a truly global variable. If its constructor requires accessing sys_params, and that needs initialising first at run time (e.g. the SetupRun(); you showed), it is your job to ensure sys_params has been initialised before a telebot is created.

jenya7

@JonB said in Sending a message to a Telegram bot:

@jenya7 said in Sending a message to a Telegram bot:

and in telebot.cpp telebot m_telebot;

Indeed, and that is where it will get instantiated.

Since you say it is m_telebot, is it a member variable in some class? Or is it a (mis-named) global? You will not want it to be a truly global variable. If its constructor requires accessing sys_params, and that needs initialising first at run time (e.g. the SetupRun(); you showed), it is your job to ensure sys_params has been initialised before a telebot is created.

I did it this way

telebot m_telebot;

static TelegramBot *bot = nullptr;

telebot::telebot(QObject *parent) : QObject(parent)
{
     //empty constructor 
}

void telebot::Start(QString api_key)
{
    bot = new TelegramBot(api_key);

    QObject::connect(bot, &TelegramBot::newMessage, this, &telebot::NewMessage);

    bot->startMessagePulling();
}

and in main.cpp

int main(int argc, char *argv[])
{
    QApplication a(argc, argv);

     MainWindow w;
     w.show();

     SetupRun();

    m_telebot.Start(sys_params.bot_api_key);
    m_telebot.SendText(sys_params.bot_chat_id, "Control board is connected");
}

This way it works good.

JonB

@jenya7
I am happy this is working for you. I will just say that you really ought not have your global telebot and TelegramBot * variables, things would be better if you used classes and instances in C++. But that's up to you. So long as telebot m_telebot is global you really ought not have that m_ prefix on the name, that is conventionally used (by some) for class member variables and will lead to confusion for anyone looking at your code, as I suspected.

jsulm

@jenya7 said in Sending a message to a Telegram bot:

This way it works good

But is really bad design as @JonB pointed out. Global variables should be avoided as much as possible...

jenya7

@JonB said in Sending a message to a Telegram bot:

@jenya7
I am happy this is working for you. I will just say that you really ought not have your global telebot and TelegramBot * variables, things would be better if you used classes and instances in C++. But that's up to you. So long as telebot m_telebot is global you really ought not have that m_ prefix on the name, that is conventionally used (by some) for class member variables and will lead to confusion for anyone looking at your code, as I suspected.

How should I use a telebot class in other modules without having it global?

jenya7

@jsulm said in Sending a message to a Telegram bot:

@jenya7 said in Sending a message to a Telegram bot:

This way it works good

But is really bad design as @JonB pointed out. Global variables should be avoided as much as possible...

I want to send messages

m_telebot.SendText(sys_params.bot_chat_id, "My messaged");

from other modules. how is it possible if m_telebot is not global?

JonB

@jenya7
I feared that you might ask this! I'm afraid I am not offering to teach/illustrate general C++ principles :) There are various alternatives, like singleton classes, even static class methods/variables, putting them into application state, doing it like QSqlDatabase does, .... I imagine that if you Google there are various suggestions proposed. I was just saying that nowadays having bald C-style global variables is "frowned upon. But also as I said, up to you.

I will make one comment. So long as this telebot m_telebot; is in your main.cpp, how do you intend to access it in other files/modules/classes anyway? I hope you're not intending to #include "main.h" in all your other .cpp files?

jenya7

@JonB said in Sending a message to a Telegram bot:

@jenya7
I feared that you might ask this! I'm afraid I am not offering to teach/illustrate general C++ principles :) There are various alternatives, like singleton classes, even static class methods/variables, putting them into application state, doing it like QSqlDatabase does, .... I imagine that if you Google there are various suggestions proposed. I was just saying that nowadays having bald C-style global variables is "frowned upon. But also as I said, up to you.

I will make one comment. So long as this telebot m_telebot; is in your main.cpp, how do you intend to access it in other files/modules/classes anyway? I hope you're not intending to #include "main.h" in all your other .cpp files?

extern telebot m_telebot; works pretty well.

JonB

@jenya7 said in Sending a message to a Telegram bot:

extern telebot m_telebot; works pretty well.

Until that module links with something which does not define a global telebot m_telebot, and then it falls over. Every module you write relies on your main.cpp defining it, not good. Again, if you want to write code where modules use extern to reference something without getting it from the appropriate header file that is up to you.

jenya7

@JonB said in Sending a message to a Telegram bot:

@jenya7 said in Sending a message to a Telegram bot:

extern telebot m_telebot; works pretty well.

Until that module links with something which does not define a global telebot m_telebot, and then it falls over. Every module you write relies on your main.cpp defining it, not good. Again, if you want to write code where modules use extern to reference something without getting it from the appropriate header file that is up to you.

I'm loosing you.

module.h

extern MY_CLASS m_my_class;

module.cpp

MY_CLASS m_my_class;

another_module.cpp

#include "module.h"

m_my_class.Method_1();

How do you think a static class works? Not the same way?

JonB

@jenya7
This is "fine", insofar as you have changed the example to some other module.cpp/.h. But your code for telebot m_telebot; I thought was in main.cpp, and that would require including main.h elsewhere. I now think did not mean it was in main, that was not clear to me.

While you choose to use a global variable, you already previously discovered (with your sys_params and TelegramBot ) that you have no opportunity to do anything initialisation-wise prior to its construction, which your code came a cropper on, you didn't understand and you had to work around. Same could happen with destructor. (You may or may not be aware, but additionally you can have a QString member (like your SYS_PARAMS sys_params) but you cannot have any QObject-derived members for such a class if you wanted them.) But it didn't put you off, so up to you.