Skip to content

Conversation

2039
Copy link

@2039 2039 commented May 24, 2020

I found it easier to do array.row(n) than array.transpose().col(n).

Note: I'm inexperienced in C/C++. I've only tested this on my own program, and it appears to be working, but it should preferably be reviewed first. Please let me know of any mistakes so I can correct them.

I found it easier to do `array.row(n)` than `array.transpose().col(n)`.

Note: I'm inexperienced in C/C++. I've only tested this on my own program, and it appears to be working, but it should preferably be reviewed first. Please let me know of any mistakes so I can correct them.
@kaskr
Copy link
Owner

kaskr commented May 25, 2020

@2039
Thanks for the suggestion, but unfortunately it's not that simple to add a row method - at least not with the current array design.

To be consistent with other classes (vectors and matrices) the array extractors must provide write access.
See e.g. here: http://kaskr.github.io/adcomp/matrix_arrays_8cpp-example.html

a1.col(1) = v1;  // assign v1 to 2nd col of a1
REPORT(a1);

This only works for array columns because columns are contiguous segments of the underlying data.

@2039
Copy link
Author

2039 commented May 25, 2020

Unless I'm missing something, arrays are already inconsistent with matrices

array<int> A(2,2);
A << 1, 2, 3, 4;
A.transpose() << 5, 6, 7, 8;
std::cout << "A:\n" << A << std::endl;
/*
A:
1
2
3
4
*/

matrix<int> M(2,2);
M << 1, 2, 3, 4;
M.transpose() << 5, 6, 7, 8;
std::cout << "M:\n" << M << std::endl;
/*
M:
5 6
7 8
*/

introducing more inconsistencies should probably be avoided, but I'd argue that the lack of a row accessor is an inconsistency by itself.

@kaskr
Copy link
Owner

kaskr commented May 25, 2020

It's true there are some inconsistencies in the array class that would be nice to fix. However, what I'm saying is that it's not so easy with the current design. I'd be happy to consider a row extractor if you make it in a way that triggers a compile time error if a user tries to assign directly to the array row. Likewise, it would be nice to fix the array transpose() caveat you pointed out, either by implementing the assignment method, or by generating a compile time error if used.

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.

2 participants