5. Longest Palindromic Substring - #110
Conversation
| class Solution { | ||
| public: | ||
| std::string longestPalindrome(std::string s) { | ||
| if (s.size() == 1){ |
There was a problem hiding this comment.
{ の前にスペースを空けることをお勧めいたします。
参考までにスタイルガイドへのリンクを共有いたします。
https://google.github.io/styleguide/cppguide.html#Horizontal_Whitespace
void f(bool b) { // Open braces should always have a space before them.
なお、このスタイルガイドは“唯一の正解”というわけではなく、数あるガイドラインの一つに過ぎません。チームによって重視される書き方や慣習も異なります。そのため、ご自身の中に基準を持ちつつも、最終的にはチームの一般的な書き方に合わせることをお勧めします。
| 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); |
There was a problem hiding this comment.
const 参照で受けているため、longest_palindrome_from() の戻り値の寿命が odd_left, odd_right の寿命まで延長されるため、コード自体は安全です。一方、このあたりの仕様はぎりぎり C++ 利用者の常識に含まれるかどうか微妙に感じます。
auto [odd_left, odd_right] で受けたほうが、読み手にとって混乱が少ないと思います。
右辺の関数が、既存のオブジェクトの参照を返すようなものであれば、問題ないと思います。
アンケート取ってみます。
https://x.com/nodchip/status/2071533267820724313
There was a problem hiding this comment.
const autoで受けた場合
`longest_palindrome_from`の結果としてpairオブジェクトが作られる
↓
それがコピーされて新たなpair型のオブジェクトとして[odd_left, odd_right]に格納される。
という認識をしていました。
無駄なコピーが発生していると思って、それを避けるためにconst参照で受けるようにしました。
寿命が odd_left, odd_right の寿命まで延長される
ことまでは意識が及んでいませんでした。
今回はpairなのでそこまで重たいオブジェクトではないですが、これが容量の大きな重たいオブジェクトを返す関数だった場合は、
既存のオブジェクトの参照を返すようなもの
にした方が良いということですかね?
There was a problem hiding this comment.
今回はpairなのでそこまで重たいオブジェクトではないですが、これが容量の大きな重たいオブジェクトを返す関数だった場合は、
既存のオブジェクトの参照を返すようなもの
にした方が良いということですかね?
設計次第だと思います。判断のポイントの一つに所有権があると思います。呼び出し先のオブジェクトに重たいオブジェクトの所有権を持たせ、呼び出し元はそれにアクセスができればよいという場合は、参照またはポインターを返せば十分だと思います。呼び出し先も呼び出し元も所有権を持ちたい場合は、呼び出し先で shared_ptr で持たせ、呼び出し元にはそのコピーを返すというやり方があります。
所有権を考えるのが面倒な場合は、オブジェクト間でやり取りされる重たいオブジェクトは shared_ptr で持ち、 shared_ptr のコピーを使ってやり取りするということも考えられます。ただ、この場合はどのオブジェクトの参照関係が分かりにくくなり、意図せず shared_ptr のインスタンスが残り、オブジェクトが破棄されず、メモリリークの原因になる可能性もあります。あまりお勧めできません。
weak_ptr の利用も考慮に入れたほうが良いかもしれません。ただし、 weak_ptr::lock() の誤用には気を付けなければなりません。
unique_ptr についても調べることをお勧めいたします。
There was a problem hiding this comment.
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();
}| } | ||
|
|
||
| private: | ||
| std::pair<int,int> longest_palindrome_from(std::string& s, int left_start, int right_start){ |
There was a problem hiding this comment.
s を非 const 参照で渡すと、読み手は s が関数内で変更されることを前提に読み始め、結果として混乱させてしまうと思います。 const 参照で渡すとよいと思います。
There was a problem hiding this comment.
まだconstや参照渡しになれていませんね。意識するようにします
| } | ||
|
|
||
| if (odd_palindrome.length >= even_palindrome.length){ | ||
| longest_palindrome = std::move(odd_palindrome); |
There was a problem hiding this comment.
std::move() すると、左辺値が右辺値にキャストされます。そのあと、 Palindrome の operator= のうち、右辺値を受けるオーバーロードが呼ばれると思います。これが明示的に実装されていないため、デフォルト実装の者が使われると思います。結果として、 std::move() なしでコピーした場合と同じような挙動になると思います。
そもそも、 int 2 つで 8 バイトしかないため、単純なコピーでも十分高速に動くと思います。
There was a problem hiding this comment.
moveの起動を詳しく理解せずに、もう使わない値を別の編集に代入する時はmoveを使うものだと思っていました。読み手が「わざわざmoveにした意図は何か」と考える負担が生まれる可能性もありそうなので、単純なコピーの方が良さそうですね。
| left--; | ||
| right++; | ||
| } | ||
| return Palindrome{left + 1, right - left - 1}; |
There was a problem hiding this comment.
return {left + 1, right - left - 1};と書くとシンプルになると思います。
https://leetcode.com/problems/longest-palindromic-substring/description/