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

Update User #1

Merged
merged 7 commits into from
Mar 4, 2017
Merged

Update User #1

merged 7 commits into from
Mar 4, 2017

Conversation

johnroyer
Copy link

there is an update of composer. and some changes below:

  • Change visibility from public to proteced in User
  • Add method toArray

I do not exactly know what method makeArray is for ? May I have an example ?
Thanks

protected $default_lang;
protected $dateOfBirth;
protected $bdayPrivary;
protected $fullName;
Copy link
Owner

@CQD CQD Jan 21, 2017

Choose a reason for hiding this comment

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

這裡的欄位不用駝峰 style 是跟著 https://www.plurk.com/API#user_data 的定義走的,一對一對應。

另外是

$genre = $user->genre;

$genre = $user->toArray()['gender'];

後者相對不是這麼容易閱讀...

如果是以避免參數被改,我建議的方式會是 magic getter 然後不寫 setter,然後把變數放在 array 裡面。不過相對 code 是沒那麼一目了然,而且這東西不會有「寫入回 plurk」的行為,所以原本沒想花力氣擋他...

@johnroyer
Copy link
Author

coding style 改回來好了。


另外關於 toArray()$user->gender 這二個算是相同的 issue:

  1. visibility 若是 public 則不需要 toArray()
  2. visibility 改成 protected / private 時,外部沒辦法直接 access class member,所以必須加上 getter 或是 toArray()__get()

我個人比較傾向改成 protected 以下,畢竟 object 傳來傳去,天小得會不會有人少打一個 = 號 XD

This reverts commit 335c01d.

Keep the variable name style in Plurk API
@CQD
Copy link
Owner

CQD commented Jan 28, 2017

那.. __get ?

我的想法還是一樣

// 這樣看起來很彆扭也不好讀
$genre = $user->toArray()['gender'];

// 這樣看起來很累贅,不如打一開始就是 array
// 但一開始就是 array 就沒辦像 $user->public_posts(舉例) 這樣用
// 而且那就跟 master 現況基本一樣(死
$user = $user->toArray();
$genre = $user['gender'];

@johnroyer
Copy link
Author

__get() 可以讓 user 抓 not visible property 時,可以先經過一些過濾,再回傳值。

例如我們不用 toArray() 時,抓值可以這樣寫:

$gender = $user->gender;  // access okay

$connection = $user->connection;  // exception, property is not exists

ps. genre 的確不好處理,但是我想你過年應該好好休息 XD

@CQD
Copy link
Owner

CQD commented Feb 2, 2017

yeap,我的意思是我推薦用 get XD

@johnroyer
Copy link
Author

那就用 __get() 下去寫吧 XD

有空在請你幫我看看這個 patch 是否和你胃口
謝啦

This reverts commit 4061631.

Use `__get()` instead of `toArray()`
excluded property `$app`
@johnroyer
Copy link
Author

我把 toArray() 拔掉了,改用 __get() 來抓 class member,另外有特別將 member $app 排除掉,他看起來不是應該用 getter 可以拉到的資料。

在麻煩有空幫我 review 一下,謝謝

@CQD CQD merged commit 45313f3 into CQD:feature/end-points-definition Mar 4, 2017
@johnroyer
Copy link
Author

甘瞎~

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.

2 participants