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

ローカル変数の取り扱い #6

Closed
shinichi-takahashi opened this issue Dec 17, 2014 · 4 comments
Closed

ローカル変数の取り扱い #6

shinichi-takahashi opened this issue Dec 17, 2014 · 4 comments
Labels
enhancement 機能追加 question Further information is requested

Comments

@shinichi-takahashi
Copy link

action()内では、カウンタ変数以外のローカル変数を許容しない
必ず、クラス変数とすること

@ttsuru
Copy link
Contributor

ttsuru commented Dec 18, 2014

初めてコメントさせていただきます。
クラス変数とはメンバ変数のことでしょうか。

全てをメンバ変数とするとメモリがオブジェクトのdestroyまで開放されず、オブジェクトのシリアライズなどにも影響する気がしますが、いかがでしょうか。
OOPの中で、本来の意味でメンバ変数とするべきものはメンバ変数で、それ以外はローカル変数にしてもよいのではないでしょうか。

あるいは、メンバ変数は必ずphpdoc形式のコメントつきでclassの先頭で宣言を行うなどはいかがでしょうか。

@izayoi256
Copy link
Contributor

issue内には記載がないのですが、
開発方針にはaction()内の規約として規定されているように読み取れます。
action()以外の関数でローカル変数を使用することに関しては問題ないのではないでしょうか。
action()からビジネスロジックを排除すれば可読性が上がりますし、
SC_FormParamのインスタンス等をメンバ変数に移動すれば
プラグインの連携もしやすくなるように思います。

@shinichi-takahashi
Copy link
Author

@ttsuru @izayoi256

コメントありがとうございます。
プラグインでローカル変数の書き換えができないという問題を解決するために、この方法でどうかとissueを立てました。
なので

開発方針にはaction()内の規約として規定されているように読み取れます。
action()以外の関数でローカル変数を使用することに関しては問題ないのではないでしょうか。

上記で@izayoi256 のおっしゃる通り、action()以外では問題ございません。
混乱を招かぬようissueの表題を変えておきますね。

また、「action()からビジネスロジックを排除すれば可読性が上が」る件に関しては、 #7 と関連が深くなってきそうなので、別でissueをたてたほうがよいかもしれません。

@Yangsin Yangsin added enhancement 機能追加 question Further information is requested labels Jan 8, 2015
@shinichi-takahashi
Copy link
Author

Silexの採用に伴い、action()がそもそもなくなったのでCloseさせていただきます。

shinichi-takahashi referenced this issue in shinichi-takahashi/ec-cube May 19, 2015
chihiro-adachi pushed a commit that referenced this issue Feb 15, 2016
geany-y pushed a commit to geany-y/ec-cube that referenced this issue May 12, 2016
kiy0taka referenced this issue in kiy0taka/ec-cube Feb 21, 2017
kiy0taka pushed a commit that referenced this issue Feb 20, 2018
…ntcontroller

PaymentControllerTestを通るように修正
kiy0taka referenced this issue in kiy0taka/ec-cube Apr 15, 2018
セッション更新の処理を追加
ndquocphong referenced this issue in onshopVN/ec-cube Oct 10, 2019
ndquocphong referenced this issue in onshopVN/ec-cube Oct 10, 2019
…re-install-plugin

Implement install plugin/theme, improve UI at listing page
okazy pushed a commit that referenced this issue Mar 4, 2020
テストを通すために最新の4.0ブランチに追従
kiy0taka added a commit that referenced this issue Jul 30, 2020
matsuoshi pushed a commit that referenced this issue Mar 16, 2021
ログイン時のカートマージでカートが別れる場合のDoctrineのエラーを回避
matsuoshi referenced this issue in matsuoshi/ec-cube Jul 5, 2021
サイトマップの改善をしました。
chrisL-meier pushed a commit to chrisL-meier/ec-cube that referenced this issue Oct 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 機能追加 question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants