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

Feature/remove endl go big or go home #1

Closed

Conversation

CJCombrink
Copy link
Owner

Applied the "cleanup" to the whole codebase

  • Also moved lonely \n and \n\n up to the previous lines
  • I see my editor stripped trailing whitespaces in many places, which I hope would not be seen as 'off topic'

- Using endl for linebreaks is bad practice
- This is confirmed by the `static const string endl = "\n"` (removed)
CJCombrink referenced this pull request Mar 11, 2024
Both the default constructor and operator== implementations reference
certain member functions of the class' members. As an example, the default
constructor references (i.e., "uses") the default constructors of its
members.

If a class contains a std::vector<Foo>, and Foo has only been *forward*-
declared (which happens often in Thrift-generated code), this creates
undefined behavior: The std::vector specification states that as long as
Foo is an incomplete type, it is fine to reference std::vector<Foo>, but
not any members (such as its default constructor).

Thus, we must defer our default constructor's implementation (which references
the default constructor of std::vector<Foo>) to a point where Foo is a
complete type. That is the case in the .cpp file.

The same holds for operator==.
@CJCombrink CJCombrink force-pushed the feature/remove_endl branch 2 times, most recently from 44c4c59 to b1abdc3 Compare March 14, 2024 21:43
@CJCombrink CJCombrink force-pushed the feature/remove_endl branch 2 times, most recently from 12cdfa5 to ac117f7 Compare March 23, 2024 10:58
@CJCombrink CJCombrink deleted the branch feature/remove_endl May 31, 2024 19:01
@CJCombrink CJCombrink closed this May 31, 2024
This pull request was closed.
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.

1 participant