ここの情報は古いです。ご理解頂いた上でお取り扱いください。

Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#2795 closed task (fixed)

kunitsuji1109 さん提供の webapp_ext/lib/db を読み込む機能について、OpenPNE側で取り込めるかどうか検討する

Reported by: ebihara Owned by: ebihara
Priority: minor Milestone:
Component: 指定しない Version:
Keywords: Cc:

Description (last modified by ebihara)

kunitsuji1109 さん提供の webapp_ext/lib/db を読み込む機能について、OpenPNE側で取り込めるかどうか検討する。

r7646, r7647 のコードについてチェックをおこない、取り込めるかどうか判断する。

取り込める場合、本線側で対応するべき事項がないか検討する。

取り込めない場合、理由を明確に示す。

Change History (6)

comment:1 Changed 12 years ago by ebihara

Description: modified (diff)
Summary: つじくにさん提供の webapp_ext/lib/db を読み込む機能について、OpenPNE側で取り込めるかどうか検討するkunitsuji1109 さん提供の webapp_ext/lib/db を読み込む機能について、OpenPNE側で取り込めるかどうか検討する

comment:2 Changed 12 years ago by ebihara

Status: newassigned

コードチェックと、本線に取り込めるかどうかの判断をさせていただきます。

comment:3 Changed 12 years ago by ebihara

Resolution: fixed
Status: assignedclosed

コード提供いただきましてありがとうございます。

申し訳ございませんが、このコードは以下に記すような事情から OpenPNE 本線に取り込むことができません。

OpenPNEの各種動作は内部的に定義された関数に強く依存しています。つまり、 OpenPNE の予期しないところで中身を書き換えられることを想定していません。

また templates, page, do ディレクトリなどと違い、本体側の関数の定義をおこなわせないという対応による影響範囲は多岐に渡り、なにかトラブルが起きた場合の原因の特定がしにくくなります。ただでさえ問題の残るバージョンアップ時のトラブルを増加させてしまう恐れもあります。 元の関数を自動的、または関数内で手動でコールする手段があればまだマシですが、このコードでは元の関数を呼ぶことができなくなってしまうのも致命的と言えます。 関数定義部分に逐一 function_exists() による判定を記述するというのは、コードの保守性を大きく低下させてしまうことでしょう。

結果としてこのコードは、OpenPNEを大きく不安定にしてしまう拡張を提供することと同義であると言わざるを得ません。拡張性に乏しい OpenPNE に柔軟な拡張性を与えるための変更なのだということは認識しており、その問題の解消の為に取り組んでいただけたことは、OpenPNE開発チームの一員として非常に感謝・評価しておりますが、しかしそれにしても安直な方法であるように思えます。

webapp_ext/lib 以下の PHP ファイルの読み込みについてのニーズは本線としても認識しておりますので、このコードを取り込むことはないとしても何かしらの対応はおこなおうと思います。

今回は、せっかく提供していただいたコードを取り込めないという結果になってしまって申し訳ありません。 今後とも OpenPNE と OpenPNE プロジェクト をよろしくお願いします。

comment:4 Changed 12 years ago by kunitsuji

Priority: criticalminor

一応検討したようですので、コメントを残して起きます。

まず採用するしないに関しては、開発する側の考え方なので、特に気にしておりません。特に採用されたいとかしてほしくないという考えもありません。

その中でコメントとして数点。 >本体側の関数の定義をおこなわせないという対応による影響範囲は多岐に渡り、なにかトラブルが起きた場合の原因の特定がしにくくなります。 この部分ですが、等改造の範囲においては、「同名のファンクションを定義して、WEB_EXTディレクトリに保存した場合」であり、 基本的に個別に改造する、もしくは独自にカスタマイズする人のための処理です。 つまり、基本処理に関しては本体構造を処理し、同名の関数をしての場所に保存した際にそちらを使うという処理となることはご理解いただいているかと思います。 ですので、改造をさせないという考え方であれば正しい選択であるかと思います。

>function_exists() による判定を記述するというのは、コードの保守性 この部分ですが、改造を受け入れる処理の場合、通常関数設定の際に「多重コール」を拒絶する際に普通に行うことかと思いますので、保守性うんぬんのもんだいではないきがします。 エラーを未然に防ぐための関数のEXISTS判定をするだけですので。 この部分に関しても、「改造をさせない」という考え方でいけば、本来多重に呼び出されることはない、ということですので、余分かもしれませんが。 EXTディレクトリを使うことでオープンソースのSNSフレームワークとして改造、改修してもらう、その上で本体のバージョンアップに影響を与えない、その上でよりよい機能やちょっとした仕組みを、フィードバックしてもらえる、という考えからウサギでは採用しました。

>しかしそれにしても安直な方法であるように思えます。 はっきりいって、これ以上の手の入れ方は存在しないように思います。 少なくとも、関数を初期時にすべて読み込んでしまう構造、クラスでオブジェクト化されているものではないので、関数名のバッティングを避けるために一瞬構造化されているように見えるが、実は保守性を思いっきり下げてしまうめいめい規則での関数名の生成。 必要な際に必要なコードをロードする、または動的に生成するという記述ができない以上、 今のコードの影響を与えずEXTを有効にするには、書かれているような「安直」な方法でしか対応できないのが今の状態ではないでしょうか?

実際にどれぐらいの影響度があるのかベンチを取って比較し、なおかつメモリの実利用量の比較を行ったうえ、安直であるこの方法でしか、実現できないということですね。

クラス化して、オートロードの仕組みを取り込むものも用意しましたが、あまりに変更点が多く、そちらのほうがPNEとしては無理ではないかという判断でこちらを提案したまでですので。

そういう意味では、3以降でしか本来の用途はなさないのかもしれません。

comment:5 Changed 12 years ago by ebihara

もしかしたら誤解を与えてしまったかもしれませんので、以下に補足します。 すでにご理解いただけていた場合、こちらの思い過ごしですので無視していただければと思います。

端的に申し上げるならば、「『現状の』OpenPNE自体、開発者、ならびにOpenPNEでSNSを運営している方々によくない影響を及ぼすコード」であると思われるため、今このコードは取り込むべきではないという判断をしました。 つまり、OpenPNEに独自の機能拡張を組み込むための機構の追加を否定したわけではありません。あくまでこのコード単体が「現状の」OpenPNEにとっては好ましくないものであるというだけのことです。

どこが好ましくないかについては前に申し上げましたので省略します。しかし、少なくないデメリットがあるにも関わらず、この方法を取ることができるMyNETSのパワーは素晴らしいと思います。

あ、それと自分が述べた「保守性の低下」の件についてですが個人的にはつじくにさんのおっしゃるような記述性の悪さについてはあまり問題視していません。本体の関数がまったく読み込まれなくなる可能性を考慮して開発をしてしまうと、たとえば共通化などの各種リファクタリングの妨げになることなどを危惧しています。

OpenPNEのコードはまだガタガタなので、リファクタリングがあまりおこなえないと、新しいバグを作る危険などがありうるのでツライところですね。

MyNETS のコードを申し訳ないことに見れていないのですが、今回いただいたコードを採用できるということは、ひとつひとつの関数が完成されている(またはそれに近い形になっている)ということなのでしょうね。正直言ってうらやましいです。

ただ OpenPNE のガタガタなコードが綺麗になるのを待っているほどに求められていない要望というわけでもないので、 OpenPNE では別のアプローチを取ることになるかなと思います。

comment:6 Changed 12 years ago by kunitsuji

MyNETS のコードを申し訳ないことに見れていないのですが、今回いただいたコードを採用できるということは、ひとつひとつの関数が完成されている(またはそれに近い形になっている)ということなのでしょうね。正直言ってうらやましいです。

に関しては、基本2.4の状態のソースと、それに対しての関数名は維持しています。。。 ただし、中に関してはりファクタリングとSQLのチューニングを行っています。 新たに機能追加したものは、関数の中に入れないでできるだけ外に「コンポーネント」としてクラス化して出しています。 必要なときにめんどくさいですがrequireするという感じです。 本当はdb/以下をすべてクラス化してメソッド名を統一して扱いたかったんですが、あまりにも作業量が多くなること、 次期バージョンへの開発を着手している現状、現行バージョンでそこまでやる必要があるのかどうか。 という判断で、現行バージョンでやれる形をとったということです。

MyNETSのコードとPNEのコードとあまり大差ありません。 あるとするとDB回リぐらいだとおもいます。

あと今回のものがMyNETSで使っているのは、 たとえば、日記の更新日時を取り出す関数があるとして、改造した場合、その関数名を変えたものを作るか、直接関数をいじるかとおもいます。 これを使うことでカスタマイズする人はWEBAPp_extに同じ名前で入れることで、目的とした処理は同一、アクションやDOから呼び出されるまでは全く同じ処理を行い、 実際の関数内の挙動のみwebapp_extを使うということになるので、逆に効率的かとおもいます。 本体の影響は受けず、自身で追加した処理(たとえば最終ログイン時間を配列に追加するとか)が本体に影響ない形で行えるということですね。

Note: See TracTickets for help on using tickets.