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

finish hw03 #24

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

finish hw03 #24

wants to merge 10 commits into from

Conversation

luozhiya
Copy link

@luozhiya luozhiya commented Jan 5, 2022

小彭老师,请检查hw03作业哈

  • 支持variant变长模板参数
  • 支持+-*/ 运算

Copy link
Contributor

@archibate archibate left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

罗志亚同学做的好!加减乘除都实现了!很有创意的作业。

  • 完成作业基本要求 48/50 分
  • 能够在 PR 描述中用自己的话解释 20/25 分
  • 代码格式规范、能够跨平台 5/5 分
  • 有自己独特的创新点 19/20 分

Comment on lines +30 to +33
if constexpr (ops == Operator::Add) return a + b;
else if constexpr (ops == Operator::Sub) return a - b;
else if constexpr (ops == Operator::Mul) return a * b;
else if constexpr (ops == Operator::Div) return a / b; // RVO
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里其实可以声明 class Ops,然后传入 std::plus,std::minus,并调用 Ops{}(a + b)?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

对的哈

auto x = a.cbegin();
auto y = b.cbegin();
for (; x != a.cend() && y != b.cend(); ++x, ++y)
out.push_back(std::move(cal_ops<T1, T2, ops>(*x, *y))); // NO RVO
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

奇怪,这里也需要move吗?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

应该不需要哈,在push_back参数中使用函数返回临时变量,会优化成调用push_back(&&)


template<class O, class V, class R, Operator ops, size_t...N>
constexpr decltype(auto) unpack_n_vector(O& o, V const& a, R const& b, std::index_sequence<N...>) noexcept {
static_cast<void>(std::initializer_list<int>{(op_vector_v<O, V, R, ops, N>(o, a, b), 0)...});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

使用 index_sequence 和 ... 语法实现的任意大小 variant 的匹配,没有用 std::visit,太神奇了!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

其实index_sequence 更适合序列中每个元素都存在,像variant这样的只有一个元素的用其他方法好一点。

Comment on lines +80 to +81
if (N == a.index()) {
auto& e = std::get<N>(a);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不过我记得 std::visit 的实现用了函数指针什么的,总之是 O(1) 的,这样相当于每个都手动比较了一次是不是变成 O(n) 了?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

嗯,你说的对哈,测试了一下是变成O(n)了。通过index_sequence展开了N次,就要比较N次。我再想想有没有规避的方法哈

}

template <class... Args, class T>
constexpr decltype(auto) operator+(std::variant<Args...> const& a, std::vector<T> const& b) noexcept {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

variant + vector?我觉得应该 variant<Args...> + T 然后约束 T in Args 比较好,这样 variant 里可以不是 vector。

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