2009-01-20 59 views
6

我剛纔寫的方法:代碼氣味? - 調整變量與+ - 1

private String getNameOfFileFrom(String path) 
{ 
    int indexOfLastSeparator = path.lastIndexOf('/'); 
    if (indexOfLastSeparator > -1) 
    { 
     return path.substring(indexOfLastSeparator + 1); 
    } 
    else 
    { 
     return path; 
    } 
} 

困擾我的是該行:

return path.substring(indexOfLastSeparator + 1); 

那是一個不好的做法,在線修改表達這樣呢?有關如何重構以提高可讀性的建議將非常受歡迎。

----編輯---- 確定更新後的評論。謝謝大家回答:) 我並不是想要真正改變變量的值,而只是使用它的表達式。

另一個帖子提示我可以在下面的第二個代碼片段中找出表達式的那部分內容。好/差/沒有區別? :)我開始懷疑我在這裏過分謹慎。

return path.substring(indexOfLastSeparator + 1); 

int indexOfFirstCharInFileName = indexOfLastSeparator + 1; 
return path.substring(indexOfFirstCharInFileName); 
+0

我不認爲有變量++的問題。我刪除了我的答案,因爲你的問題是語言不可知的。但是,如果你正在寫作。NET,你應該使用System.IO.Path類作爲你的文件路徑解析邏輯。 – Will 2009-01-20 14:24:37

+0

++運算符在這裏不相關,因爲變量本身沒有被修改。 – 2009-01-20 14:26:40

+0

正在迴應某人刪除了他們的評論... – Will 2009-01-20 14:28:51

回答

4

如果你想要去的整套方法,我會做這樣的事情:

int indexOfLastSeparator = path.lastIndexOf('/'); 
    if (indexOfLastSeparator > -1) 
    { 
      int indexOfFirstCharInFilename = indexOfLastSeparator + 1; 
      return path.substring(indexOfFirstCharInFilename); 
    } 
    else 
    { 
      return path; 
    } 

這還不具有良好的性能,因爲創建一個新的變量,但它的方式高度可讀的我認爲你期望它。

個人而言,我寧願像約翰·諾蘭建議:

return path.substring(++indexOfLastSeparator); 

這不是健談像你想的那樣,但即使是初學編程得到的想法,如果他讀的子方法的說明。

編輯:請注意,在路徑檢查中使用「/」不是平臺無關的。如果你的代碼應該在不同的平臺上運行,使用lib函數來獲取與系統相關的分隔符。

4

在這種情況下,你可以做:

private String getNameOfFileFrom(String path) 
{ 
     return path.substring(path.lastIndexOf('/') + 1); 
} 

至少,在Java和.NET,x.substring(0)會返回一個字符串相等。在Java中,它將返回相同的參考 - 不確定.NET。

一般來說,有很多次增加或減少一個是有意義的。我真的不認爲這是一種代碼味道。

0

如果您使用.NET看看System.Io.Path.Combine(string,string)這是使用以2串而不檢查和/或刪除終端結合斜線

或者你可以使用string.Remove(string.LastIndexOf("/"));

+0

如果路徑的第二部分是根(「\ this \ path \ is \ rooted」),則Combine的行爲方式您通常不會期望。 – Will 2009-01-20 14:25:52

7

我不明白爲什麼這樣做是一個不好的做法。也許可以在該行之前增加變量名稱並將其與新值一起使用。

但除此之外,我沒有看到一個問題

2

我沒有與你在做什麼有什麼問題,但我認爲你可以使用一個註釋,使絕對清楚地表明你尋找最後/並在該位置之後佔用字符串的其餘部分。

0

你沒有修改那裏的變量。

要做到這一點是

return path.substring(indexOfLastSeparator++); 

return path.substring(++indexOfLastSeparator); 
+0

而這些選項中,只有第二個會提供你想要的結果(增量後的運算符將返回原始變量)。 – 2009-01-20 14:27:42

+0

這是正確的Phill。 – 2009-01-20 14:33:48

+0

對不起,我沒有試圖mydify變量,只有表達式 – willcodejavaforfood 2009-01-20 14:45:34

1

只要是容易閱讀和理解的代碼,我沒有看到這種技術的任何問題。如果你想增加一個額外的行中的變量,它實際上可能變得不太可讀。

0

你不修改變量本身,你只修改表達式。這完全沒問題,特別是在這種情況下,你希望你的新字符串以之後的字符開頭,最後一個分隔符的位置。

+0

我可能可以改善我的標題的措辭 – willcodejavaforfood 2009-01-20 14:44:53

2

實際上在.net中,您應該使用Path.GetFilename();來代替。

0

It IS in GDI + or drawing code。

有時候一個潛在的問題會導致東西被某個像素左右偏離,此時您有兩個選擇。

  • 解決潛在的問題
  • 使所有其他操作使用蹩腳的+1或-1的偏移。

如果後者被選中,這段代碼的氣味會遍佈整個代碼。

2

有兩個原因更喜歡在一個神奇的數字寫一個常數:

  1. 如果該值改變,重構用一個神奇的數字
  2. 幻數減少可讀性的時候可以是一個噩夢。

如果你確定這些都不是問題,我說它去。在你給作爲示例代碼的情況下,我認爲:

  1. 的從最後分離路徑的偏移是不可能改變和
  2. 這並不難告訴發生了什麼事情。

所以我說這是一個幻數的有效使用。不過,我也要指出,你不必擔心這兩個問題比你更多,所以如有疑問,請使用常量。

0

在Java path.substring(n)中不修改路徑,甚至不修改路徑引用的String對象。