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

Further amendment to use new/delete to handle vector<string> #1726

Merged
merged 2 commits into from
Jul 26, 2023

Conversation

christopherlam
Copy link
Contributor

I guess the _main_matcher_info should be inited via new/delete instead of g_new0/g_free because of the vector.

@christopherlam christopherlam force-pushed the stable-progress branch 2 times, most recently from 40c4596 to 8919796 Compare July 25, 2023 16:20
@jralls
Copy link
Member

jralls commented Jul 25, 2023

I don't think this is a good idea without at least writing constructors and a destructor and getting rid of the coy typedef. It really should be the last step of a thorough rewrite into real C++ instead of "better C".

@christopherlam
Copy link
Contributor Author

It's too difficult right now to safely convert to c++ so I'd prefer to use GList instead of vector<Transaction*>

@christopherlam christopherlam force-pushed the stable-progress branch 5 times, most recently from b14e749 to 586dfee Compare July 26, 2023 14:19
@christopherlam
Copy link
Contributor Author

christopherlam commented Jul 26, 2023

I think it's much simpler to revert recent changes and move a few instructions in gnc_gen_trans_list_add_trans_internal which should does achieve the same result of fast imports.

because there's a better strategy, in next commit
This will speed up both importing new transactions, and destroying
existing ones.
@code-gnucash-org code-gnucash-org merged commit 4172468 into Gnucash:stable Jul 26, 2023
3 checks passed
@christopherlam christopherlam deleted the stable-progress branch July 26, 2023 15:22
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.

3 participants