ETL: Refactor MetaData #988
Conversation
| var numReplicas: Short, | ||
| var topicConfig: Map[String, String] | ||
| ) { | ||
| protected def this() = this( |
There was a problem hiding this comment.
請問這個預設的建構式有使用情境嗎？理想上最好是讓每個建構式跑完後的物件都有存在的用途
There was a problem hiding this comment.
沒有使用情景，它只是爲了讓build pattern能夠執行的方式。
我也感覺這個東西怪怪的，但找了幾個網站scala寫build parttern都是這樣，我暫時也沒想到替代的方式。
還是我自己賦予他一個存在的用途？
There was a problem hiding this comment.
Builder pattern的話，我們只會在最後才真的建構出「有使用目的」的物件，換言之，就是從程式碼端就預防沒意義或不合法的物件被建構出來。
| new Metadata(this) | ||
| } | ||
|
|
||
| def topicConfig(map: Map[String, String]): Metadata = { |
There was a problem hiding this comment.
這邊的話比較適合將 Builder 明確建構出來，也就是說 setter 都應該在 Builder 內部發生
There was a problem hiding this comment.
因為 metadata 是一個 pojo ，是一個使用者會傳來傳去使用的物件，現在的做法會導致我們無法得知一個metadat 是否已經可用、是否已經建構完成
There was a problem hiding this comment.
另外，現在這個寫法也會讓Metadata 變成不是 immutable，這個在現在分散式多執行緒的環境下會不容易使用
There was a problem hiding this comment.
另外，現在這個寫法也會讓Metadata 變成不是 immutable，這個在現在分散式多執行緒的環境下會不容易使用
意思就是儘量讓傳遞的物件都爲immutable的，保證安全。以後會注意
|
已添加builder，再麻煩學長繼續review。 |
| import java.io.File | ||
|
|
||
| class MetadataBuilder private ( | ||
| var deploymentModel: String, |
There was a problem hiding this comment.
要加上private，否則外部都可以直接變更他的變數
|
|
||
| def deploymentMode(str: String): MetadataBuilder = { | ||
| this.deploymentModel = str | ||
| new MetadataBuilder(this) |
There was a problem hiding this comment.
這種手法會在call chain 裡面產生兩個 builder，很容易發生誤會，例如
var builder;
setSomething(builder.set(xxx));上面的第二行在設定好某個變數後，順便傳給某個方法去設定builder其他東西，但因為這個builder會在每次setter的時候傳回一個新的builder，因此傳給setSomething的builder已經跟原本的builder是不同物件，這就導致setSomething所做的設定通通無法傳遞給原本的builder
簡而言之，要避免這種用法引免產生很隱性的bug
There was a problem hiding this comment.
好 我修改成了return this，避免了這種奇怪的寫法
|
|
||
| import java.io.File | ||
|
|
||
| class MetadataBuilder private ( |
There was a problem hiding this comment.
這個物件應該要放在object Metadata底下，這樣呼叫者才可以寫出Metadata.builder這種常用的呼叫方式
|
內容已修正，再麻煩學長review。 |
|
@wycccccc 好像跟你另一隻PR產生衝突了，麻煩看一下 |
|
已解決衝突 |
|
@wycccccc 可否先修正一下失敗的測試？我看了最近main branch常出現ETL相關的錯誤 |
|
#917 我將該issue一起放在這裡解決了，感覺上也能稱的上refactor metadata，我還能少解些衝突。 |
這些錯誤都只在github上跑的時候才出現，我有點難以下手，我再想想辦法。 |
|
@wycccccc 可否rebase一下，看看測試是否穩定了 |
我嘗試跑了兩次，看上去沒什麼問題了 |
之前的MetaData寫的感覺亂亂的，因此抽空refactor了一下。
將每個配置參數是否允許爲空，及默認值抽了出來。
並將對應配置參數的處理方式進行了整理，方便未來添加新參數及閱讀程世碼。