有没有更优雅的方式来编写这个CoffeeScript？

这工作，但确实有任何建议为写这更简单的/优雅的方式？：有没有更优雅的方式来编写这个CoffeeScript？

if @mediaAddQueue[''+mid]['mediaType'] is 'photo' and 
    @mediaAddQueue[''+mid]['econ_status'] is 'loaded' and 
    @mediaAddQueue[''+mid]['medium_status'] is 'loaded' and 
    @mediaAddQueue[''+mid]['thumb_status'] is 'loaded' and 
    not @mediaAddQueue[''+mid]['targetEventRecord']? 
    @addMedia @mediaAddQueue[''+mid]['targetEventRecord'], mid, @mediaAddQueue[''+mid]['mediaType'] 
else if @mediaAddQueue[''+mid]['mediaType'] is 'video' and 
    @mediaAddQueue[''+mid]['video_status'] is 'loaded' and 
    not @mediaAddQueue[''+mid]['targetEventRecord']? 
    @addMedia @mediaAddQueue[''+mid]['targetEventRecord'], mid, @mediaAddQueue[''+mid]['mediaType']

来源

2013-02-07 celwell

“是否有更优雅的方式来编写此CoffeeScript？” - 当然，用C重写：P – 2013-02-07 22:43:57

当然！首先，请注意@mediaAddQueue[''+mid]遍布整个地方，您可以用一个变量替换它。此外，如果该属性具有有效的标识符作为名称，则不需要访问something['prop']等属性;你可以做something.prop。更改这两件事情已经清理代码颇有几分：

media = @mediaAddQueue[mid] 

if media.mediaType is 'photo' and 
    media.econ_status is 'loaded' and 
    media.medium_status is 'loaded' and 
    media.thumb_status is 'loaded' and 
    not media.targetEventRecord? 
    @addMedia media.targetEventRecord, mid, media.mediaType 
else if media.mediaType is 'video' and 
    media.video_status is 'loaded' and 
    not media.targetEventRecord? 
    @addMedia media.targetEventRecord, mid, media.mediaType

然后，通知，无论是if和else if里面的代码是一样的。我认为如果你能够给条件赋予一些有意义的名字，这样代码就会变得更加自我记录和DRY，那将是非常好的。我不太了解这段代码的上下文，所以我会猜测变量名称;尝试尽可能清楚使用的东西，解释它的意义可能：

media = @mediaAddQueue[mid] 

isValidPhoto = media.mediaType is 'photo' and 
    media.econ_status is 'loaded' and 
    media.medium_status is 'loaded' and 
    media.thumb_status is 'loaded' and 
    not media.targetEventRecord? 

isValidVideo = media.mediaType is 'video' and 
    media.video_status is 'loaded' and 
    not media.targetEventRecord? 

if isValidPhoto or isValidVideo 
    @addMedia media.targetEventRecord, mid, media.mediaType

来源

2013-02-07 23:40:53 epidemian

promanow在我写我的时候加了他的答案。他建议使用函数来干代码非常有用，IMO。如果功能对某些功能来说是非常本地的，我倾向于使用变量......但是如果它需要在不同的地方使用，请不要三思而后行：寻求功能！ = D – epidemian

这是正确的 - 功能很少是一个**糟糕的主意。 JavaScript/CoffeeScript使它们成为语言的基础，鼓励尽可能多地使用它们;） – pawroman

非常感谢，非常感谢。 – celwell

当然是。总有东西可以重构。

临时变量

@mediaAddQueue[''+mid]

无处不在。考虑通过引入一个临时的辅助变数重构：

elem = @mediaAddQueue[''+mid]

的代码会突然变得更具可读性。

功能，功能，功能！

我看你执行特定类型的检查（我认为我们有elem变量）：

elem['econ_status'] is 'loaded' and 
elem['medium_status'] is 'loaded' and 
elem['thumb_status'] is 'loaded'

你可以写一个函数，这样elem（或者不管这个对象是）并执行此检查，其参数是对象，要比较的值和对象的键。感谢splats，这在Coffee中非常简单。

## 
# Check whether all obj's keys are set to value. 
checkAllKeys = (obj, value, keys...) -> 
    for k in keys 
     if obj[k] != value 
      return false 

    return true

现在前面的代码块将成为：

checkAllKeys(elem, 'loaded', 'econ_status', 'medium_status', 'thumb_status')

如果你知道你将检查 '装' 值的时候，让自己的另一个功能：

checkLoaded = (elem, keys...) -> 
    checkAllKeys(elem, 'loaded', keys...)

如果'econ_status', 'medium_status', 'thumb_status'键通常会一起检查，那么即使是一个更多的功能可能是一个好主意：

checkPhotoLoaded = (photo) -> 
    checkLoaded(photo, 'econ_status', 'medium_status', 'thumb_status')

我的重构规则是应该为所有重复两次以上的东西写一个函数。 CoffeeScript使写作功能变得有趣而且快速。

我希望这有帮助。

来源

2013-02-07 23:37:53 pawroman

迄今为止最优雅的功能答案;}） –

从epidemian答案去，我仍然会重构它有点这样的：

media = @mediaAddQueue[mid] 

typeValidationMap = 
    'photo' : (m) -> 
    m.econ_status is 'loaded' and 
    m.medium_status is 'loaded' and 
    m.thumb_status is 'loaded' 
    'video' : (m) -> 
    m.video_status is 'loaded' 
    'default':() -> no 

isValidMedia = (m) -> 
    return no if m.targetEventRecord? 
    validate = typeValidationMap[m.mediaType] or typeValidationMap.default 
    validate m 

if isValidMedia media 
    @addMedia media.targetEventRecord, mid, media.mediaType

侧面说明：我注意到你传递在这种情况下总是为null的targetEventRecord

来源

2013-02-08 17:13:00 Guillaume86

谢谢。有趣和灵活的想法。 – celwell

有没有更优雅的方式来编写这个CoffeeScript？

回答

相关问题