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

Modernize: use auto wherever possible for object initialisation #407

Open
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

krishnakumarg1984
Copy link
Collaborator

@krishnakumarg1984 krishnakumarg1984 commented Jun 29, 2023

The C++ core guidelines, a living document co-authored by Stroustrup and Sutter, states in ES.11 that:

ES.11: Use auto to avoid redundant repetition of type names

Reason

  • Simple repetition is tedious and error-prone.
  • When you use auto, the name of the declared entity is in a fixed position in the declaration, increasing readability.
  • In a function template declaration the return type can be a member type.
Example

Consider:

auto p = v.begin();      // vector<DataRecord>::iterator
auto z1 = v[3];          // makes copy of DataRecord
auto& z2 = v[3];         // avoids copy
const auto& z3 = v[3];   // const and avoids copy
auto h = t.future();
auto q = make_unique<int[]>(s);
auto f = [](int x) { return x + 10; };

In each case, we save writing a longish, hard-to-remember type that the compiler already knows but a programmer could get wrong.

@mmcleod89
Copy link
Contributor

I personally disagree that auto makes code more readable. It can do, under some circumstances: as an example I think sometimes it makes for loops easier and cleaner where the type iterated over is obvious. In most code though, especially assignments, it can make it much easier for people who want to read and maintain the code to be able to clearly see correct type. In these cases, it is obvious because of the casts on the right hand side (but this also makes any errors obvious), but in your example code above it obfuscates the code. What is the type of h = t.future()? If I want to use this in my code and understand it, I now have to look up this function, instead of just reading right where it is declared. This spreading out of dependency makes existing code bases harder to get to grips with.

@mmcleod89
Copy link
Contributor

To be clear, I do think that auto can in principle help to prevent accidental implicit casts (which I assume are the errors referrered to when they describe manual typing as "error prone"), but I don't think this is a common problem in practice, and I disagree with the conclusion to use auto "wherever possible". For the changes made here I think it's fine because it's always clear what the type is anyway, but it also doesn't really make much of a difference: it doesn't correct any code that was wrong and I don't think that the resulting code is any more readable than before.

@krishnakumarg1984
Copy link
Collaborator Author

krishnakumarg1984 commented Jul 3, 2023

@mmcleod89, at least for things like:

Eigen::ArrayBase<T0> &coeffs = const_cast<Eigen::ArrayBase<T0> &>(coeffs_);
Eigen::ArrayBase<T1> &signal = const_cast<Eigen::ArrayBase<T1> &>(signal_);

we could strongly consider auto to avoid the redundancy with long data types. The following is more readable in my opinion.

auto &coeffs = const_cast<Eigen::ArrayBase<T0> &>(coeffs_);
auto &signal = const_cast<Eigen::ArrayBase<T1> &>(signal_);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants