Giter Club home page Giter Club logo

Comments (24)

fnc12 avatar fnc12 commented on May 23, 2024

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

from sqlite_orm.

fnc12 avatar fnc12 commented on May 23, 2024

@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

from sqlite_orm.

DonaldTrump88 avatar DonaldTrump88 commented on May 23, 2024

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

from sqlite_orm.

fnc12 avatar fnc12 commented on May 23, 2024

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.

from sqlite_orm.

DonaldTrump88 avatar DonaldTrump88 commented on May 23, 2024

I just did pull request. Did you receive it?

from sqlite_orm.

fnc12 avatar fnc12 commented on May 23, 2024

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

from sqlite_orm.

DonaldTrump88 avatar DonaldTrump88 commented on May 23, 2024

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
  1. Renaming Storage.replace to insert_with_id to avoid confusion and understand usage from name.
  2. Renaming Storage.insert to insert_make_id to avoid confusion and understand usage from name.

from sqlite_orm.

fnc12 avatar fnc12 commented on May 23, 2024
  • 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?
  1. Storage::replace is named replace cause SQLite has query REPLACE. Details here
  2. 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.

from sqlite_orm.

fnc12 avatar fnc12 commented on May 23, 2024

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

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

?

from sqlite_orm.

DonaldTrump88 avatar DonaldTrump88 commented on May 23, 2024

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

from sqlite_orm.

fnc12 avatar fnc12 commented on May 23, 2024

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

from sqlite_orm.

DonaldTrump88 avatar DonaldTrump88 commented on May 23, 2024

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.

from sqlite_orm.

DonaldTrump88 avatar DonaldTrump88 commented on May 23, 2024

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?

from sqlite_orm.

fnc12 avatar fnc12 commented on May 23, 2024

@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

from sqlite_orm.

fnc12 avatar fnc12 commented on May 23, 2024

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

from sqlite_orm.

fnc12 avatar fnc12 commented on May 23, 2024

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

from sqlite_orm.

DonaldTrump88 avatar DonaldTrump88 commented on May 23, 2024

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);
}

from sqlite_orm.

DonaldTrump88 avatar DonaldTrump88 commented on May 23, 2024

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

from sqlite_orm.

fnc12 avatar fnc12 commented on May 23, 2024

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

from sqlite_orm.

DonaldTrump88 avatar DonaldTrump88 commented on May 23, 2024

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 ?

from sqlite_orm.

fnc12 avatar fnc12 commented on May 23, 2024

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?

from sqlite_orm.

DonaldTrump88 avatar DonaldTrump88 commented on May 23, 2024

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

from sqlite_orm.

fnc12 avatar fnc12 commented on May 23, 2024

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

from sqlite_orm.

fnc12 avatar fnc12 commented on May 23, 2024

closing due to inactivity

from sqlite_orm.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.