2009-01-20 47 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对象。