Skip to content

5. Longest Palindromic Substring - #110

Open
kazuki-official wants to merge 1 commit into
mainfrom
5-longest-palindromic-substring
Open

5. Longest Palindromic Substring#110
kazuki-official wants to merge 1 commit into
mainfrom
5-longest-palindromic-substring

Conversation

@kazuki-official

Copy link
Copy Markdown
Owner

Comment thread memo.md
class Solution {
public:
std::string longestPalindrome(std::string s) {
if (s.size() == 1){

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://google.github.io/styleguide/cppguide.html#Horizontal_Whitespace

void f(bool b) { // Open braces should always have a space before them.

なお、このスタイルガイドは“唯一の正解”というわけではなく、数あるガイドラインの一つに過ぎません。チームによって重視される書き方や慣習も異なります。そのため、ご自身の中に基準を持ちつつも、最終的にはチームの一般的な書き方に合わせることをお勧めします。

Comment thread memo.md
int longest_length = 0;
int longest_palindrome_start = 1;
for (int i = 0; i < s.size() - 1; i++){
const auto& [odd_left, odd_right] = longest_palindrome_from(s, i, 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.

const 参照で受けているため、longest_palindrome_from() の戻り値の寿命が odd_left, odd_right の寿命まで延長されるため、コード自体は安全です。一方、このあたりの仕様はぎりぎり C++ 利用者の常識に含まれるかどうか微妙に感じます。
auto [odd_left, odd_right] で受けたほうが、読み手にとって混乱が少ないと思います。
右辺の関数が、既存のオブジェクトの参照を返すようなものであれば、問題ないと思います。

アンケート取ってみます。
https://x.com/nodchip/status/2071533267820724313

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.

const autoで受けた場合

`longest_palindrome_from`の結果としてpairオブジェクトが作られる
↓
それがコピーされて新たなpair型のオブジェクトとして[odd_left, odd_right]に格納される。

という認識をしていました。

無駄なコピーが発生していると思って、それを避けるためにconst参照で受けるようにしました。

寿命が odd_left, odd_right の寿命まで延長される

ことまでは意識が及んでいませんでした。

今回はpairなのでそこまで重たいオブジェクトではないですが、これが容量の大きな重たいオブジェクトを返す関数だった場合は、

既存のオブジェクトの参照を返すようなもの

にした方が良いということですかね?

@nodchip nodchip Jun 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

今回はpairなのでそこまで重たいオブジェクトではないですが、これが容量の大きな重たいオブジェクトを返す関数だった場合は、

既存のオブジェクトの参照を返すようなもの
にした方が良いということですかね?

設計次第だと思います。判断のポイントの一つに所有権があると思います。呼び出し先のオブジェクトに重たいオブジェクトの所有権を持たせ、呼び出し元はそれにアクセスができればよいという場合は、参照またはポインターを返せば十分だと思います。呼び出し先も呼び出し元も所有権を持ちたい場合は、呼び出し先で shared_ptr で持たせ、呼び出し元にはそのコピーを返すというやり方があります。
所有権を考えるのが面倒な場合は、オブジェクト間でやり取りされる重たいオブジェクトは shared_ptr で持ち、 shared_ptr のコピーを使ってやり取りするということも考えられます。ただ、この場合はどのオブジェクトの参照関係が分かりにくくなり、意図せず shared_ptr のインスタンスが残り、オブジェクトが破棄されず、メモリリークの原因になる可能性もあります。あまりお勧めできません。
weak_ptr の利用も考慮に入れたほうが良いかもしれません。ただし、 weak_ptr::lock() の誤用には気を付けなければなりません。
unique_ptr についても調べることをお勧めいたします。

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.

https://www.geeksforgeeks.org/cpp/auto_ptr-unique_ptr-shared_ptr-weak_ptr-in-cpp/

上記サイトで確認してきました。

C++ではオブジェクトの寿命管理をするためにスマートポインタが定義されていて、その代表的なものが、unique_ptr, shared_ptr, weak_ptrなのですね。

呼び出し先のオブジェクトに重たいオブジェクトの所有権を持たせ、呼び出し元はそれにアクセスができればよいという場合は、参照またはポインターを返せば十分だと思います

というのは以下のような利用だと考えました。

class Owner {
private:
    A a_obj;

public:
    A& getA() {
        return a_obj;
    }

    A* getAPointer() {
        return &a_obj;
    }
};

int main() {
    Owner owner;

    A& a_ref = owner.getA();
    A* a_ptr = owner.getAPointer();
}

また、

所有権を考えるのが面倒な場合は、オブジェクト間でやり取りされる重たいオブジェクトは shared_ptr で持ち、 shared_ptr のコピーを使ってやり取りするということも考えられます

というのは以下のような利用だと考えました。

std::shared_ptr<A> f1() {
    return std::make_shared<A>();
}

void main () {
    std::shared_ptr<A> p = f1();
}

Comment thread memo.md
}

private:
std::pair<int,int> longest_palindrome_from(std::string& s, int left_start, int right_start){

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

s を非 const 参照で渡すと、読み手は s が関数内で変更されることを前提に読み始め、結果として混乱させてしまうと思います。 const 参照で渡すとよいと思います。

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

まだconstや参照渡しになれていませんね。意識するようにします

Comment thread memo.md
}

if (odd_palindrome.length >= even_palindrome.length){
longest_palindrome = std::move(odd_palindrome);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

std::move() すると、左辺値が右辺値にキャストされます。そのあと、 Palindrome の operator= のうち、右辺値を受けるオーバーロードが呼ばれると思います。これが明示的に実装されていないため、デフォルト実装の者が使われると思います。結果として、 std::move() なしでコピーした場合と同じような挙動になると思います。
そもそも、 int 2 つで 8 バイトしかないため、単純なコピーでも十分高速に動くと思います。

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.

moveの起動を詳しく理解せずに、もう使わない値を別の編集に代入する時はmoveを使うものだと思っていました。読み手が「わざわざmoveにした意図は何か」と考える負担が生まれる可能性もありそうなので、単純なコピーの方が良さそうですね。

Comment thread memo.md
left--;
right++;
}
return Palindrome{left + 1, right - left - 1};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

return {left + 1, right - left - 1};

と書くとシンプルになると思います。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants