Skip to content

Create memo.md#3

Open
ntanaka1984 wants to merge 2 commits into
mainfrom
1.-Two-Sum
Open

Create memo.md#3
ntanaka1984 wants to merge 2 commits into
mainfrom
1.-Two-Sum

Conversation

@ntanaka1984
Copy link
Copy Markdown
Owner

  1. Two Sum を解きました。
    問題: https://leetcode.com/problems/two-sum/description/
    言語:C++
    お手すきのときにレビューのほど何卒お願い申し上げます。

Comment thread 1. Two Sum/memo.md
public:
vector<int> twoSum(vector<int>& nums, int target) {
std::map<int, int> num_to_index;
for (int i = 0;i < nums.size();i++) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

for (int i = 0; i < nums.size(); i++)

とスペースを開けるのが一般的ではないでしょうか。
https://google.github.io/styleguide/cppguide.html

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

色々な流儀がありますので、自分の考えと理由を持ち、周りに合わせる、くらいが期待でしょう。

https://docs.google.com/document/d/11HV35ADPo9QxJOpJQ24FcZvtvioli770WWdZZDaLOfg/edit?tab=t.0#heading=h.9xjrags8izok

http://www.libjingu.jp/trans/bs_faq2-j.html#coding-standard
Bjarne Stroustrup (C++開発者によるお勧めスタイルガイド)

おすすめのコーディング標準はありますか
C++コーディング標準の主な目的は、特定の環境の中で特定の目的を達成するために、C++の使用方法に関する規則を定めることです。したがって、あらゆる使用目的とすべてのユーザに適したコーディング標準がひとつしかないということはありえません。
優れた規則がありますが、その規則は主としてメタ規則であると考えてください。
優れた標準というものは、定期的にレビューされ、経験と作業環境における変化をもとに更新されるものです。

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

お二人共コメントありがとうございます。リンク先の資料も読んで自分なりのスタイルを固めていきたいと思います。今のところセミコロンの後ろを開けると初期化部、条件部、インクリメント部がはっきりと区別できるメリットがあると感じました。

Comment thread 1. Two Sum/memo.md
public:
vector<int> twoSum(vector<int>& nums, int target) {
std::map<int, int> num_to_index;
for (int i = 0;i < nums.size();i++) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

評価値を使わないのであれば ++i の方がよいです。

https://google.github.io/styleguide/cppguide.html#Preincrement_and_Predecrement

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

https://docs.google.com/document/d/11HV35ADPo9QxJOpJQ24FcZvtvioli770WWdZZDaLOfg/edit?tab=t.0#heading=h.dwdqi73ych8d
https://discord.com/channels/1084280443945353267/1350090869390311494/1364158499118125128

ここまでの理屈を前提にしたうえで、私が、ここまでしつこく書いているのは、「前置を好む」は常識ではないにしても「上の理屈が通じる」くらいが常識なので、「過程を飛ばして結論だけ覚えていると常識がない」と見えるからです。逆に「結論を知らなくても過程が通じればよほど常識がある」と判断されるでしょう。

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

ありがとうございます。後置だとインクリメント前と後の2つのデータを持っておかないといけないため、コピーが生じてデータによっては無駄にコストがかかるのですね。理由もよく理解できました。iterator の実装にも興味が湧いてきたので手が空いたらやってみたいと思います。

Comment thread 1. Two Sum/memo.md
int rest_num = target - nums[i];
if (num_to_indices.contains(rest_num)) {
std::vector<int> rest_num_indices = num_to_indices.at(rest_num);
for (int j: rest_num_indices) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

j のあとにスペースを空けることをおすすめします。

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

コメントありがとうございます。google のc++ スタイルガイドでもコロンの両脇にスペースを入れるのが推奨されていますね。
https://google.github.io/styleguide/cppguide.html#Horizontal_Whitespace

Comment thread 1. Two Sum/memo.md
for (int i = 0;i < nums.size();i++) {
int rest_num = target - nums[i];
if (num_to_indices.contains(rest_num)) {
std::vector<int> rest_num_indices = num_to_indices.at(rest_num);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

rest_num_indices の内容を変更しないので、 const 参照で受け、コピーが発生しないようにしたほうが、余計な処理が減り、軽くなると思います。

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.

4 participants