Skip to content

完成 Issue13#15

Open
G100my wants to merge 8 commits intoMontyPan:masterfrom
G100my:issue13
Open

完成 Issue13#15
G100my wants to merge 8 commits intoMontyPan:masterfrom
G100my:issue13

Conversation

@G100my
Copy link
Copy Markdown
Collaborator

@G100my G100my commented Dec 7, 2019

No description provided.

@MontyPan MontyPan self-requested a review December 8, 2019 02:17
Copy link
Copy Markdown
Owner

@MontyPan MontyPan left a comment

Choose a reason for hiding this comment

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

你這個版本實在太樸實無華且枯燥了,但是你不是有錢人,所以我不接受。

Must FIX

  • 就算是我能接受這個版本,也得ii 拔掉,實在太多餘了
    • 我們通常不會在 for-loop 裡頭搞兩個以上的 index 值,這會讓程式很難讀,寧願改用 while-loop。
  • 請生出更厲害的版本(開始 v2 / v3 吧...... 😱 )

Comment thread basic/src/main/java/lo/basic/Issue13.java
Comment thread basic/src/main/java/lo/basic/Issue13.java
@G100my G100my requested a review from MontyPan December 8, 2019 18:58
Copy link
Copy Markdown
Owner

@MontyPan MontyPan left a comment

Choose a reason for hiding this comment

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

Must FIX

我在 這個 comment 已經講了,這裡就不再贅述。以後給這種程度的「不同版本」是無效的,更別說有些改完之後還更爛 [扶額]。

請砍掉 v2 / v3,重新再搞一個版本,而且要更好而不是更差的。

瘟腥洨蹄擤:請善用前提。


還有,(不只這個 PR)該是你 resolve 的 conversation 請自動自發... 😪

@G100my
Copy link
Copy Markdown
Collaborator Author

G100my commented Dec 9, 2019

不知道這樣改有沒有比較好= =+
(感覺改完跟 v2 還是很像啊 orz)

另外請問您指的比較厲害的版本,是主要要求 簡短 易讀 好理解 這點嗎?

@G100my G100my requested a review from MontyPan December 9, 2019 09:12
@MontyPan
Copy link
Copy Markdown
Owner

MontyPan commented Dec 9, 2019

你要暗示我業障重,貼這張圖就可以了,不要搞一個國王的 commit 來要我點評。


至於「什麼叫做比較厲害的版本」這個問題,既然你問了,那倒是挺值得回答的。

通常我們判斷一個程式的厲害與否,大概是這幾個面向:

  • 精簡 / 簡短
  • 執行效率好
  • 可讀性高
  • 彈性
  • robust(我覺得中文翻起來反而沒那個味,就還是使用原文 XD)

彈性跟 robust 你現在寫的程式可能碰不到(這個 issue 完全碰不到),要說的話 #11 的 optional question 算是起點。

比較慘的是,在 94.87% 的情況下,這幾個面向是互斥、很難全部滿足。像「精簡」跟「可讀性」就很難 國共一家親 並存。

這也是我一開始只用「厲害」這種模糊形容詞的原因,算是自由發揮、看你能搞出什麼名堂這樣。當然,如果你仔細思考的話,這個 issue(或說這類的題目)通常只會在意其中一個面向。

anyway... 我想現在的重點是,不管哪個面向,都很難說你目前的 v2 / v3 有比 v1 好...... 😱

@MontyPan MontyPan removed their request for review December 9, 2019 13:59
@G100my G100my requested a review from MontyPan December 9, 2019 14:49
Copy link
Copy Markdown
Owner

@MontyPan MontyPan left a comment

Choose a reason for hiding this comment

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

我真的不知道該說什麼了,所以只好把問題丟回去給你。

Must Answer

請說明為什麼你會覺得 v4 有比 v1 好?

Must FIX

請改出「更」厲害的 v5

@G100my
Copy link
Copy Markdown
Collaborator Author

G100my commented Dec 12, 2019

Must Answer

請說明為什麼你會覺得 v4 有比 v1 好?

當初認為以 隨著 forLoop 運行次數而往 data 後方遞增的 ii ,
減去 只有在從 data 提取的內容不一樣的時候而改變的 i ,
來計算相同 data 內容的數量,可以讓每次迴圈改變的變數數量少一個,
少了每次隨 forLoop 迴圈而變動的 counter。

@MontyPan
Copy link
Copy Markdown
Owner

當初認為以 隨著 forLoop 運行次數而往 data 後方遞增的 ii ,
減去 只有在從 data 提取的內容不一樣的時候而改變的 i ,
來計算相同 data 內容的數量,可以讓每次迴圈改變的變數數量少一個,
少了每次隨 forLoop 迴圈而變動的 counter。

OK,我接受這個答案,不過不是因為內容正確性與否(而且坦白說我看不太懂, 連 v4 的程式碼也是),而是你 居然 的確說了一套看起來能對得上的理由,當然前提是我沒誤會 & 腦補太多的話...... 🙈

我不是很想仔細探究 v4(因為實在很可怕),我簡單提出一個觀點:你在 v1 用了四個變數、你在 v4 也還是用了四個變數,然後你在 v4 裡頭少了一個改變的變數 counter,但我其實看不太出來 counter 跟 i 有什麼決定性的差異。

你的意思應該是說在 v1 的話 counter 是一定會改變,但是到了 v4 的話是符合條件才改變,所以 v4 比 v1 好? OK,這的確有道理,但實際上我們會忽略這種... 量級(這個詞不太好,但是我找不到其他的詞)的優化,因為那不是必然發生。

倒是 v4 的缺點非常顯而易見,光 L45 / L48 的重複就散發著濃烈的壞味道。


我不知道你是又忘了按 re-request 還是怎麼的,只是這兩天我沒什麼耐心,所以我直接把 v5 也說一說。你的 v5 是有 bug 的,請找出來(我懷疑不只一個但是我沒有證據)。然後這有沒有比 v1 好可能很難定論(我個人是覺得半斤八兩),不過可以肯定的是,你沒有妥善用到前提假設,所以沒辦法製造出很明顯的改善。

@MontyPan
Copy link
Copy Markdown
Owner

畢竟是公開 repo 然後你講了一些個人隱私的事情,所以上一個 comment 我先砍了

Repository owner deleted a comment from G100my Dec 14, 2019
@G100my
Copy link
Copy Markdown
Collaborator Author

G100my commented Dec 16, 2019

目前找到兩個 v5 的 Bug:

  • Bug-1 是如果是第一個數字 data[0] 數量最多的話,計算會錯誤,數量會比實際數量少 1,跳過了一次計算。
    解決方式為 把 counter +=1 位置往上放,讓程式先執行 counter +1 再進行判斷。
  • Bug-2 是如果 data 內的數字數量都是 1,這樣的話並不會進入第二個 ifnumber 也不會記錄任何數字,執行結果會是初始值 0 。同樣情況也發生在 v1max_counter 預設為 0,但因為數字數量如果都是 1,內層的 if 判別不會為 Truemax_counter 就會維持在預設值。
    解決方式為把預設值改為 data[0]

對於前提假設(已經排序過、數字不會亂跳、相同數字靠在一起的 array)
我還是有點想不出要怎麼妥善運用 orz,
但還是自己先生出一個可讀性感覺比較好一點且比較直覺思考的 v6


不小心把原本 untracked file 也加進去了= =+
(想說怎麼顯示 completed with 2 local objects......)

@G100my G100my requested a review from MontyPan December 17, 2019 01:37
Copy link
Copy Markdown
Owner

@MontyPan MontyPan left a comment

Choose a reason for hiding this comment

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

Must FIX

不小心Issue16 給弄上來,然後因為有規定說不能作 force update 那也就算了,那你 至少也設法補救 阿幹,不然是要我 accept 那個 Issue16

Requirement Change

我沒有很正式地導入 testing 的概念(一方面是我的確很少在實務上搞 testing...... 😱,二方面是也不要一次塞太多東西 ),不過既然都到這個地步,你都炸了 bug 而且你也找到了 bug,那應該是要針對你發現的 bug 搞個 test case,不然怎麼知道你有沒有改正確 & 以前的 test case 有沒有炸掉?

然後 test case 一多還要用肉眼去校正就會很阿雜,所以程式設計師應該想辦法偷懶,這裡先隨便舉個例子:

private static boolean test(int[] value, int[] answer) {
    return value[0] == answer[0] && value[1] == answer[1];
}

test(longest(data1), answer1);
test(longest(data2), answer2);

當然,這仔細看會覺得還是有點蠢,不過目前你手邊的工具就只能作到這樣 XD,但即便如此還是會爽上一點?

請試試看(甚至你要補強上面的 test() 都行)然後補上你發現的 bug 的 test case。

murmur

我還是有點懷疑你的 v6 有 bug(至少味道不太好,第一次迴圈根本無意義阿,有需要用到四個變數?),但是我真的沒有證據...... 💃

目前看起來你是擠不出什麼新鮮東西了,請把上面的 requirement change 改完(然後他 X 的不要忘記 must fix)然後先去作 #16 吧....

ps.1 我的確沒發現 v1 版本有 bug,這也佐證了 testing 很多時候比 code review 還實在
ps.2 程式設計師要偷懶,是在作法上偷懶,而不是在想法上偷懶。

@G100my G100my requested a review from MontyPan December 17, 2019 12:23
Copy link
Copy Markdown
Owner

@MontyPan MontyPan left a comment

Choose a reason for hiding this comment

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

這個 task 我想就比照 task-0 就先在這裡告一段落但是不 close,如果你哪天頓悟了有新的想法也可以回來 try 一下。


Bewared

  • 刪除檔案應該要獨立一個 commit(基本上你之前也常常有應該拆成兩個 commit 但是卻擠在一起,不過那還有商量的空間,現在這個就沒得商量了)
  • 通常我們不會留修正前的程式啦(commit 都有阿),目標是讓 test case 都 pass 而不是印出一堆東西來(哪有時間看)。
    • 萬一你還有回頭來改這個 task 的話再砍砍掉吧

@G100my
Copy link
Copy Markdown
Collaborator Author

G100my commented Dec 17, 2019

如果你哪天頓悟了有新的想法也可以回來 try 一下。

好的!我也覺得過段時間回頭想,很有可能會有新的思考邏輯出現。

@MontyPan MontyPan mentioned this pull request Jan 5, 2020
@G100my
Copy link
Copy Markdown
Collaborator Author

G100my commented Jan 23, 2020

結果過了好一陣子了還是想不出個新想法= =+
幾乎就跟 v1 一樣啊Q_Q
v7 只差在 如果兩個數一樣的話會立即跳開進入下一次的 for 迴圈,
不會每次都確認。另外為求保險,後面又加上 ‵if(counter > max)‵ 來避免最大數量的數字是最後一組時,數值沒被記錄到。
呵呵 有重複程式碼在,應該沒關係吧XD...?
這應該已經是我能想到執行步驟最少的版本了Q_Q

關於 瘟腥洨蹄擤,我對於前提的解讀是 數值從小排到大。
.... 你應該沒有陰險到把「最後一個明顯大於其他數值」的 65535,也列入前提條件中吧....

@MontyPan
Copy link
Copy Markdown
Owner

我就當上面那個 comment 是 request review 了...

我先承認,v7 我懶得看..... 倒是你的 L118 不用仔細看也知道有 94.87% 的機率是沒意義 or 可以取代的。

至於你「懷疑溫馨小提醒」的段落,說實在我完全不懂你在講啥。有哪裡會讓你覺得「傳入的陣列,最後一個 element 一定 會遠大於前面 element」的錯覺?就算會好了,那又能怎麼樣?


有鑑於你似乎變不出什麼新把戲了,所以你可以先不用再製造新的版本,改思考這個問題吧!

Must Answer

你是依照 longest() 的 JavaDoc 寫程式的,可以說那兩行字就是這個案子的需求。

好的,我當初故意埋了一個哏在裡頭,有可能 會產生問題,請試著找出問題是什麼? 💃 (謎之聲:WTF... 總共也才幾個字為什麼以能埋哏阿阿阿阿....)

@G100my
Copy link
Copy Markdown
Collaborator Author

G100my commented Feb 4, 2020

有可能 會產生問題

data 是 已經排序過的 array,
這個吧,因為有可能會被打亂或者修改

@MontyPan
Copy link
Copy Markdown
Owner

MontyPan commented Feb 4, 2020

=.= 請不要修改前提假設,前提假設就是會傳給 callee 一個已經排序過的 array。

我倒是比較好奇你是基於什麼回答「可能會被打亂或著修改」......

@G100my
Copy link
Copy Markdown
Collaborator Author

G100my commented Feb 4, 2020

用平常不會覺得他是問題的角度找梗到底在哪裡Q_Q

@MontyPan
Copy link
Copy Markdown
Owner

MontyPan commented Feb 4, 2020

==" 還好我用中性的問句,所以問出了一些驚人的事實(?)

就字面上來說,「有可能會被打亂或者修改」的確是我們在寫程式要考慮的點,主要是 caller 要預防,callee 要按耐住不要惡搞。在 #21 那個 try-try-see 的問題解答其實就是這句。完整的版本是:

因為(除了 primitive data type 之外)call by reference 的關係,如果都是使用同一個 reference,
所以有人改變了 array(object 亦同,其實 array 本來也就是 object)的內容,每個用相同 reference 都會受到影響。

所以你在 #21 就會有可能發生「程式莫名其妙出現 bug 但是怎麼查也查不出來」的狀況,因為就結果來說,caller 可以改變你的 stack 值(失去封裝的保護)

這邊你先看過有個印象即可,有緣我們再來回頭講這個(因為需要蠻多基礎知識)。


回歸主題(?):

  • 即使「有可能會被打亂或著亂改」是對的,對 callee 來說,你要自毀前提假設那是你家的事情,跟題目無關。至於 caller 他傳 array 之前就應該要預防,更別說拿出 SRP 大刀一揮「caller 的死活關我屁事」
  • 題目一開始就明確的問你「需求埋了一個哏」,完全沒要討論實作面的事情。

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