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.

Sign up for GitHub

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

Jump to bottom

[C++] Change std::vector<std::shared_ptr<Field>> to use it's alias: FieldVector for maintainability #36950

Closed
jsjtxietian opened this issue Jul 31, 2023 · 1 comment · Fixed by #37101
Closed

[C++] Change std::vector<std::shared_ptr<Field>> to use it's alias: FieldVector for maintainability #36950

jsjtxietian opened this issue Jul 31, 2023 · 1 comment · Fixed by #37101
Assignees
@jsjtxietian
Labels
Component: C++ good-first-issue Type: enhancement
Milestone
14.0.0

Comments

@jsjtxietian
Copy link
Contributor

jsjtxietian commented Jul 31, 2023 •
edited
Loading

Describe the enhancement requested

In type_fwd.h, we have a type alias declaration:
using FieldVector = std::vector<std::shared_ptr<Field>>

But some of the function declaration in this file use this alias while some don't.
Maybe it's a good idea to change all std::vector<std::shared_ptr<Field>> to FieldVector in type_fwd.h and even files like type.cc for clarity and maintainability ?

Component(s)

C++
@jsjtxietian
Copy link
Contributor Author

I can take this issue if no one has started working on it yet.

@github-actions github-actions bot mentioned this issue Aug 10, 2023
Merged
@pitrou pitrou added this to the 14.0.0 milestone Aug 14, 2023
pitrou pushed a commit that referenced this issue Aug 14, 2023
@jsjtxietian
GH-36950: [C++] Change std::vector<std::shared_ptr<Field>> to use it'…
9cabd94 
…s alias: FieldVector (#37101)

### Rationale for this change

Clarity and Maintainability

### What changes are included in this PR?

1. Changed all occurrence of std::vector<std::shared_ptr<Field>> in **type_fwd.h**、**type.h** and **type.cc** to FieldVector for consistency.
2. Use move to eliminate vector copy in sparse and dense union's constructor

### Are these changes tested?

Covered by existing tests

### Are there any user-facing changes?

No

* Closes: #36950

Authored-by: jsjtxietian <jsjtxietian@outlook.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
loicalleyne pushed a commit to loicalleyne/arrow that referenced this issue Nov 13, 2023
@jsjtxietian @loicalleyne
apacheGH-36950: [C++] Change std::vector<std::shared_ptr<Field>> to u…
83445a3 
…se it's alias: FieldVector (apache#37101)

### Rationale for this change

Clarity and Maintainability

### What changes are included in this PR?

1. Changed all occurrence of std::vector<std::shared_ptr<Field>> in **type_fwd.h**、**type.h** and **type.cc** to FieldVector for consistency.
2. Use move to eliminate vector copy in sparse and dense union's constructor

### Are these changes tested?

Covered by existing tests

### Are there any user-facing changes?

No

* Closes: apache#36950

Authored-by: jsjtxietian <jsjtxietian@outlook.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jsjtxietian jsjtxietian

Labels
Component: C++ good-first-issue Type: enhancement
Projects
None yet
Milestone
14.0.0
3 participants
@felipecrv @pitrou @jsjtxietian