失敗的代碼審查

今天我的代碼失敗，它的代碼審查的原因之一，而不是使用的Request.QueryString（「noClicks」）我已經使用的Request.QueryString（「noClicks」）的toString，審查不合格它作爲了的toString多餘的，我爭辯說，tostring只是默認的，它並沒有增加代碼的權重（request.querystring行的兩個實例每頁調用一次被調用一次）....他們是小事，還是這樣的事情你應該擔心在代碼審查，我只問對我來說tostring只是明確的，我不會擔心失敗的代碼審查

來源

2011-04-07 spacemonkeys

我不知道他們在dotNet世界中說什麼，但在Java中我傾向於使用toString （）明確地提醒自己發生了什麼事情。所以是的，我會同意這是完全微不足道的。不會有開銷或速度的影響，它不會影響可讀性。

（我是從VB6世界來到最初並用默認寬鬆，但我必須說，我認爲這是最好是明確可能的情況下）。

所以是的，我不會擔心正常。

一個好的代碼審查討論的正確性，良好的封裝/模塊化，風格，和實施的質量，而不是小語法糖的事情。

來源

2011-04-07 21:01:32 MJB

謝謝大家，同意所有評論，就像這篇文章末尾的總結 – spacemonkeys 2011-04-07 21:20:46

良好的代碼審查使代碼更好。可讀性是其中的一部分。因爲你在這裏基本上做的事情就像'AMethod（「hello world」.ToString（））''一樣，所以在這裏沒有「語法糖」。是的，ToString（）調用不會減慢任何關係。但它仍然是令人敬畏的。 – tster 2011-04-07 21:30:00

你誇大其辭。你的例子當然是一個愚蠢的騙子。雖然這個人有意識地在對象上明確調用toString（）並不總是很清楚，但我已經看到這是一個問題。無論如何，沒有一個人正確的思想會讓某人失去一些微不足道的東西。他們可能會建議改變它，當然。 – MJB 2011-04-09 04:07:12

這看起來很愚蠢。我可以理解，如果它們對於顯式表達式是狂熱的，並且你寫了Request.Querystring（「noClicks」）而不是Request.Querystring（「noClicks」）。ToString，因爲這不是必然總是提供預期的工作其他對象時的行爲。

這就是說，他們有在你訪問一個字符串集合點。如果它是List（Of String），那麼調用：yourList [0] .ToString看起來很愚蠢。這基本上就是你在這裏完成的。這很挑剔，但我可以看到這一點。

來源

2011-04-07 21:01:58

從功能角度來看，我不會擔心它。

從可讀性/ asthetics（SP）的角度來看，的ToString（）上漲到一個字符串的末尾是難看。

來源

2011-04-07 21:02:05

坦率地說，這對我來說似乎很小。一方面，ToString是多餘的，他應該指出這一點。另一方面，它非常小，我不確定它不會被優化。你確定這是唯一的原因嗎？

來源

2011-04-07 21:02:42 dlev

記住，那就是，代碼審查是部分存在，以確保每個人都遵守相同的風格。如果spacemonkeys是一種編碼方式，而我正在編碼另一種方式，則會使代碼更難流通。

因此，它可能是「失敗」，只是因爲缺乏對「規則」的遵守。

我曾經目睹了爭執

if(condition)

或

if (condition)

是否更好。在大多數情況下，它的臉頰和幽默很有趣，但這些事情往往發生在代碼評論中。

來源

2011-04-07 21:04:45 taylonr

夠正確。即使有簡明的標準和慣例文件，我也見證了不止一場神聖的戰爭，而是通過簡單的格式或命名問題。 – 2011-04-07 21:07:25

我不得不同意其他評論，它似乎很少...使用.ToString（）雖然有一個問題。如果密鑰不存在，它將拋出一個空異常。

來源

2011-04-07 21:04:47 RandomWebGuy

這是一種代碼味道，需要刪除。這完全沒用，只會讓代碼難以閱讀。

來源

2011-04-07 21:06:18 tster

真的很難閱讀，其中request.querystring（）返回一個對象，它只是不是提醒讀者，request.querystring（「名稱」）不？ – spacemonkeys 2011-04-07 21:09:23

request.querystring（「name」）返回一個字符串。如果你把.ToSting（）放在那裏，這讓我覺得它沒有。然後當我看到它的時候，我想知道ToString（）在那裏做什麼。現在我浪費了幾秒鐘的時間來了解ToString，而不是理解代碼。 – tster 2011-04-07 21:18:19

+1。你的評論指出它。可讀性至關重要。 QueryString [always]（http://msdn.microsoft.com/zh-cn/library/system.web.httprequest.querystring.aspx）返回一個字符串。讀者輸入不必要的轉換令人困惑。我在自己的代碼評論中標記了類似的內容，我將繼續這樣做。 – MarkJ 2011-04-08 10:10:29

由於這個原因，我不會讓代碼審查失敗，但是您不信任Querystring方法返回字符串的錯誤信號。

ToString的使用應該僅限於有意義的情況，並且因爲該方法可以應用於每個對象，所以這會引起我的懷疑。包含不必要的方法調用也不是一個好代碼的標誌，因爲它要求讀者讀取更多的標記。

來源

2011-04-07 21:08:20

失敗的代碼審查

回答

相關問題