fix some issues of Array2 #61
Conversation
| @@ -112,21 +105,20 @@ struct Array2 { | |||
| } | |||
|
|
|||
| IndexT size1; | |||
| IndexT size2; // the number of elements in the array, equal to | |||
There was a problem hiding this comment.
The initialization order of data members in the initialization list of the constructors should match the definition order.
There was a problem hiding this comment.
Actually I prefer to keep the original order (size1, indexes, size2, data) here as it's more readable (indexes is related to size1, and data is related to size2).
There was a problem hiding this comment.
Different people have different opinions about what is more readable.
The current approach is more memory-efficient.
There was a problem hiding this comment.
Will merge and leave it with unresolved. We can update it later if other guys (@danpovey) think we should change the order in the initialization list
|
+2 |
|
I agree with haowen, more memory efficient.
…On Mon, Jun 22, 2020 at 12:30 PM Haowen Qiu ***@***.***> wrote:
Merged #61 <https://github.com/danpovey/k2/pull/61> into master.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<https://github.com/danpovey/k2/pull/61#event-3466436107>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAZFLO3BNY7QWLSV3VO6CHDRX3M6BANCNFSM4OEGGXQA>
.
|
I am confused. |
It is a best practice to let the initialization order match the definition order, though the current implementation is fine. |
|
Oh sorry.. Make it match to avoid warnings is what i would do.
…On Monday, June 22, 2020, Fangjun Kuang ***@***.***> wrote:
The initialization order of data members in the initialization list of the
constructors should match the definition order.
It is a best practice to let the initialization order match the definition
order, though the current implementation is fine.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<https://github.com/danpovey/k2/pull/61#issuecomment-647441260>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAZFLO7NPJ7FQKCUWO6O6JDRX4ZV5ANCNFSM4OEGGXQA>
.
|
|
There's no any warning in current code. |
No description provided.