Countributions

[DonaldTrump88]

I made some changes to the code. This is first time that I am using github.
Can you anyone tell me procedure to commit code?
Or I can send code to the developers and they can merge it?

[fnc12]

You forked the lib - this is correct. Next step - modify code and commit & push it. Next go to your fork page on github and press Send pull request button. I'll receive your pull request and accept or deny it. Thanks

[fnc12]

@Ninja-007 btw you can discuss improvements here. Please tell me what do you miss in the lib and I'll assist you or improve it. Thanks

[DonaldTrump88]

Thanks for your reply. It is very nice.

1. Usage of cout directly in library is not good. In my application, only application supposed to do cout. The library can use callback function(registered by application during initialisation) for logging. If logging is enabled, then application can log the messages from lib else can ignore it. It is upto application to decide which messages to display.
2. Usage of uint8_t, uint16_t, uint32_t and int8_t as datatypes. I have some classes which I cannot change and datatype of classmembers should reflect allowed range. (I added code for this.)
3. Replacing std::to_string with std::stringstream usage or better alternative. On some compliers the function is not present. ((I added code for this.)
   Regards

[fnc12]

Oh thanks. Please push your changes to your repo and create a pull request. Points 1 and 3 are very good. But there is a thing about point 2. uint8_t are not language types but typedefs so we'd better add unsigned data types support so uint8_t, uint16_t, uint32_t and int8_t will be supported too.

[DonaldTrump88]

I just did pull request. Did you receive it?

[fnc12]

@Ninja-007 yes I did. Thanks. You did right. Let me inspect it a little bit and then I'll write you back.

[DonaldTrump88]

New requests. If you have alternative, then add write here.

1. Utilty functions

• bool IsDBPresent(const std::string& DBPath) //will verify presence of DB on disk
• bool Storage.IsTablePresent(const std::string& TableName) //will verify presence of table on in given storage intance(returned by make_storage)
• bool UserClass.IsRowPresent(T primarykey) //verifies presence of row in UserClass attached table

2. Renaming Storage.replace to insert_with_id to avoid confusion and understand usage from name.
3. Renaming Storage.insert to insert_make_id to avoid confusion and understand usage from name.

[fnc12]

• bool IsDBPresent(const std::string& DBPath) why? What are you going to perform is db exists? What if not?
• bool Storage.IsTablePresent(const std::string& TableName) same question. I assume sync_schema is enough
• bool UserClass.IsRowPresent(T primarykey) why? What use cases do you expect with it?

2. Storage::replace is named replace cause SQLite has query REPLACE. Details here
3. Storage::insert doesn't create id in all cases - it created id if primary key column is int. But if primary key is std::string for example it doesn't. To be exact Storage::insert returns last insert row id. So I don't think we need to rename these functions. Replace needs primary key anyway. So then insert is the only way to insert without primary key.

[fnc12]

@Ninja-007 why do you use unary plus operator before adding object to stream like this:

std::stringstream stream;
stream << +t;
return stream.str();

?

[DonaldTrump88]

To get numerical values instead ASCII characters
https://stackoverflow.com/questions/19562103/uint8-t-cant-be-printed-with-cout

[fnc12]

@Ninja-007 wow nice way. I do static_cast in this cases. Ok I leave it.

[DonaldTrump88]

Thanks,
I have to write some default values in table if it is initilized for first time. When table is present, then nothing should be done. This should be done before doing sync_schema.
BTW, I wrote IsDBPresent and IsTablePresent, if you ready to integrate then I can push the code.

[DonaldTrump88]

I checked you recent checkin.
There is code like
// char is neigher signer char nor unsigned char so it has its own specialization
template<>
struct field_printer {
std::string operator()(const signed char &t) const {
std::stringstream stream;
stream << +t;
return stream.str();
}
};

You have wrote signed char in function defination. Is that expected?

[fnc12]

@Ninja-007 default values can be added with default_value constraint in make_column function. Then call sync_schema and:

• table will be added if it doesn't exist with with DEFAULT in specified columns
• noting will be done if table already exists with the same schema
• table will be altered or recreated if table exists with different schema

About field_printer's char specialization:
yes, I missed argument type in operator(). Thanks

[fnc12]

There is no need in IsDBPresent and IsTablePresent - storage is designed to be static

[fnc12]

@Ninja-007 what do you mean IsDBPresent? What does it returns? Whether file exists? Or whether all tables exist with a given schema?

[DonaldTrump88]

I added simple function which checks for existence of file.
/*!
\fn IsDBPresent
\brief verifies presence of database file on disk.
\warning the file may not be valid sqlite database.
*/
static bool IsDBPresent(const std::string& DBPath);
{
struct stat buffer;
return (stat (DBPath.c_str(), &buffer) == 0);
}

[DonaldTrump88]

I am made pull request to fix complier warnings. Warnings in library are not good.

[fnc12]

checking file existence can be implemented in file system libraries: boost fs or it's C++17 standard analog. Or one can use libc for this with FILE *file = fopen("path.db"); . Also file system interaction differs on different operating systems and this is not about sqlite at all. Also there can be any other file (image probably) with the same name just accidentally but IsDBPresent returns true and this is not correct cause this file has nothing common with database. sync_schema is a better way to perform file and schema check.

Warnings will be fixed. Thanks

[DonaldTrump88]

You have very good library. But I donot find some many forks or users of this library.
Would you mind to submit this lib to https://github.com/fffaraz/awesome-cpp ?

[fnc12]

Thanks. I'd like to. I've already tried to submit there about two or three times. But pull request wasn't accepted with no comments. Maybe you should try?

[DonaldTrump88]

You should raise issue for that. I am not sure about pull request. Once you raise issue, let me know. I shall comment there.

[fnc12]

Looks like sqlite_orm got its place in awesome cpp repos. Are there any other issues within it? @Ninja-007

[fnc12]

closing due to inactivity