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

Feature/#79 看板發文限制以及細部資訊 #80

Merged
merged 3 commits into from
May 1, 2021
Merged

Feature/#79 看板發文限制以及細部資訊 #80

merged 3 commits into from
May 1, 2021

Conversation

nickyanggg
Copy link
Contributor

@nickyanggg nickyanggg commented Apr 24, 2021

👏 解決掉的 issue / Resolved Issues

⛏ 變更內容 / Details of Changes

  • 在 bbs.go 中新增 BoardRecordSettings、BoardRecordInfo
  • 在 pttbbs/board_test.go 中新增 TestBoardHeaderInfo、TestBoardHeaderSettings

@y2468101216
Copy link
Contributor

LGTM

@PichuChen
Copy link
Member

實作方式這樣有問題,可以直接把設定值export到 interface 去而不是回傳 map 這樣嗎?

interface 的部分不一定通通要擠在一個大的 interface 裡面,可以稍微拆分成不同類別的 interface

@y2468101216
Copy link
Contributor

實作方式這樣有問題,可以直接把設定值export到 interface 去而不是回傳 map 這樣嗎?

interface 的部分不一定通通要擠在一個大的 interface 裡面,可以稍微拆分成不同類別的 interface

這樣的話直接回傳 struct 更好吧?

@PichuChen
Copy link
Member

實作方式這樣有問題,可以直接把設定值export到 interface 去而不是回傳 map 這樣嗎?
interface 的部分不一定通通要擠在一個大的 interface 裡面,可以稍微拆分成不同類別的 interface

這樣的話直接回傳 struct 更好吧?

他本身就是回傳 struct 了。更精確來說他本來是回傳 interface BoardRecord

所以需要定義的是一個 BBS 的 BoardRecord 哪些 Setting 值是一定會有需要實作的,哪些是分開定 Interface 的,那這樣在應用上的話可以用 b, ok := ret.(xxxInterface) 這樣知道這個 BBS 系統的看板有實作這些設定值

@nickyanggg
Copy link
Contributor Author

實作方式這樣有問題,可以直接把設定值export到 interface 去而不是回傳 map 這樣嗎?

interface 的部分不一定通通要擠在一個大的 interface 裡面,可以稍微拆分成不同類別的 interface

可能要再麻煩幫忙 review 一下是不是有符合上面的意思,感謝。

另外想問一下改成這種方式的確感覺會更加彈性且合理,但原本回傳 map 的實作方式會在哪一段遇到問題呢?

@PichuChen
Copy link
Member

實作方式這樣有問題,可以直接把設定值export到 interface 去而不是回傳 map 這樣嗎?
interface 的部分不一定通通要擠在一個大的 interface 裡面,可以稍微拆分成不同類別的 interface

可能要再麻煩幫忙 review 一下是不是有符合上面的意思,感謝。

另外想問一下改成這種方式的確感覺會更加彈性且合理,但原本回傳 map 的實作方式會在哪一段遇到問題呢?

map 的話會在效能上有問題,不過比較大的問題應該還是在慣例上。通常在這部分並不會用 map 去做處理。

另外是Map 的話Key值的部分假如未來要修改時因為是字串,會容易出現Bug, 目前的 method name 算是比較暫時的,因為沒有一個文件去描述為什麼是這些method name

Copy link
Member

@PichuChen PichuChen left a comment

Choose a reason for hiding this comment

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

LGTM

@PichuChen PichuChen merged commit 0a4883f into Ptt-official-app:development May 1, 2021
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.

[主線] [PTT] 實作看板發文限制以及細部資訊
3 participants