current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 1 down vote favorite

I have two very similar functions that exist mainly for syntactic sugar.

deactivate = (el, seek) -> el.find(seek).removeClass("active")
activate = (el, seek) -> el.find(seek).addClass("active")

This allows me to write, for example:

activate $("#something"), ".child"

Using the above purely as an example, ignoring any logical errors with the JavaScript itself… how could this be refactored to be more dry?

share|improve this question

2 Answers 2

up vote 2 down vote accepted

You can use js' bracket notation for this task

alter = (el, seek, method) -> el.find(seek)[method + "Class"]("active")
#use like this:
alter $("#something"), ".child", "add"

However, your situation doesn't really call for DRYness. Your method names make more sense than alter, or whichever name you may choose.

share|improve this answer
    
You could just use your alter function as a stepping stone for the definitions of activate/deactivate become more DRY: deactivate = (el, seek) -> alter el, seek, "remove" activate = (el, seek) -> alter el, seek, "add" (Disclamer: I don't use CoffeScript) –  RoToRa Dec 19 '11 at 13:56
    
@RoToRa Correct, and you can also do it the other way around: Store activate and deactivate on an object, and pass which one you want as an argument to alter: containingObject[method]("active") –  Zirak Dec 19 '11 at 14:05
    
Thank you. I think the syntactic benefit of my initial functions outweigh the benefit of making the code more DRY. I was just wondering if there was something I was missing here. –  Toast Dec 19 '11 at 20:43
up vote 0 down vote

I think your code is fine. I would only suggest a more complex solution only if this pattern happens more often.

For example, if you want to refactor to allow for other class names you can do something like this:

adder   = clsName -> (el, seek) -> el.find(seek).removeClass(clsName)
remover = clsName -> (el, seek) -> el.find(seek).addClass(clsName)

activate   = adder 'active'
deactivate = remover 'active'
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.