"DRY" programming in JASS?

[HungryHusky]

So I have been taking some programming lessons recently and learned of the concept "DRY" (do not repeat yourself). Which basically means avoid using the same function over and over again.

Example:

DRY

function Trigger_Actions_GetRandomInt takes integer lowBound, integer highBound returns integer
    return GetRandomInt(lowBound, highBound)
endfunction
function Trigger_Actions takes nothing returns nothing
    local integer intOne = Trigger_Actions_GetRandomInt(0, 5)
    local integer intTwo = Trigger_Actions_GetRandomInt(0, 5)
    local integer intThree = Trigger_Actions_GetRandomInt(0, 5)
endfunction

Not DRY

function Trigger_Actions takes nothing returns nothing
    local integer intOne = GetRandomInt(0, 5)
    local integer intTwo = GetRandomInt(0, 5)
    local integer intThree = GetRandomInt(0, 5)
endfunction

Does DRY not apply in JASS? Since I've seen many JASS coders don't use the concept of DRY. And which alternative is better?

 

[Jampion]

In this case it makes no sense to use Trigger_Actions_GetRandomInt instead of GetRandomInt, because both do exactly the same. Trigger_Actions_GetRandomInt is one additional function call so it's slower. That's why many users mostly use natives. If you write your own function it should offer a lot more to be worth using it.
If you would for example make your GetRandomInt to have a higher probability to take numbers close to the average of lowBound and highBound, then it would be complicated enough, so it would make sense to use a custom function.
I mean in your case it makes no difference if you repeat Trigger_Actions_GetRandomInt(0, 5) or GetRandomInt(0, 5), but if Trigger_Actions_GetRandomInt(0, 5) would be 10 lines long, it would make a lot of sense to use Trigger_Actions_GetRandomInt(0, 5) instead of these 10 lines everytime.

 

[Chaosy]

For it to be worth it you need at least two lines in the function you call over and over.
You basically just made a bad BJ function, which is banned in jass for good reason.

 

[IcemanBo]

This is more a topic of redundance than dry.

dry is more:

function a{
-- do_some_a()
-- do_some_b()
-- do_some_c()
}

function b{
-- do_some_a()
-- do_some_b()
-- do_some_c()
-- do_some_d()
}

.... ^we want to call function "b", there is nothing redundant, but we come in conflict with dry -- we should do:

function b{
--call a()
-- do_some_d()
}

as we repeat the same logics, so we in some cases can centralize it and only do the logics in function a. But this also makes only sense to some extend.

Edit:

You should not repeat yourself when it makes sense to connect content logicaly.
But on the other side you want autonomous code pieces, which don't need logics of a "god class" or such thing, for every single line.
Somewhere between spaghetti and ravioli code is the good. : )

 

[aple]

Coming from someone who isn't really a programmer in the slightest, even to me this concept sounds like a pretty bad practice. You shouldn't force yourself to make sure you don't repeat because you'll just end up writing very inefficient code in an attempt to make sure nothing repeats. I guess in a case like JASS (Or really any other scripting language, I guess) it doesn't totally matter because you're not writing literally hundreds of thousands, to potentially even millions, of lines of code to build a fresh program from the ground up, but in that hypothetical case where you are doing that I think it would be a pretty bad idea to try and stick to that philosophy to the T.

Maybe if you're trying to learn how everything works, this concept can be useful.

 

[Kaijyuu]

What I always learned was "do not copy/paste code". Any time you find yourself repeating a series of actions (or a similar series of actions), try to instead make a function call that does it in 1 line. This makes code maintenance easier, since any updates will only have to be done once instead of everywhere. Something newbies sometimes fall into would be failing to use loops and/or recursion, which is sort of the same idea too. Redundant code is bad. It makes maintenance a headache.

 

[Dr Super Good]

I do not think the example is correct. I would imagine they should be...

DRY

function Trigger_Actions_GetRandomInt takes nothing returns integer
    return GetRandomInt(0, 5)
endfunction
function Trigger_Actions takes nothing returns nothing
    local integer intOne = Trigger_Actions_GetRandomInt()
    local integer intTwo = Trigger_Actions_GetRandomInt()
    local integer intThree = Trigger_Actions_GetRandomInt()
endfunction

Not DRY

function Trigger_Actions takes nothing returns nothing
    local integer intOne = GetRandomInt(0, 5)
    local integer intTwo = GetRandomInt(0, 5)
    local integer intThree = GetRandomInt(0, 5)
endfunction

In the DRY example the procedural coupling from the function arguments is removed. Why this is good is that the range of the random numbers can now be maintained in just 1 place, instead of 3 and potentially even more.

I personally try to do this whenever possible. However be aware that it is slightly detrimental in JASS due to the lack of optimizing compilers. Where as in C++ and Java there is a good chance the function call will be automatically in-lined for no overhead, in JASS it remains a function call which the JASS virtual machine performs. This is not a concern for most code but performance intensive code, eg a projectile system, should rather use the less maintainable but more efficient non DRY form for optimization. JASS is one of those languages where one has to trade maintainability to optimize.

 

[HungryHusky]

@Chaosy

Yes this is what I suspected but I was unsure.

@Dr Super Good

Yes your example is correct. Your explanation made alot of sense. Thanks!

 

[EdgeOfChaos]

DRY absolutely applies in JASS. But that example is not good DRY. That is making unneeded functions which is just as bad as repeating yourself.

Here's a real example of DRY.
When we need to cast a dummy ability, there are usually 5 lines we use.
1) Create a dummy unit
2) Add ability to dummy unit
3) Order dummy unit to cast
4) Add expiration to dummy unit
5) Null dummy unit
5 lines doesn't sound like much, but if you use TONS of dummy abilities, it'll be very annoying to CnP this every time.
What I usually do if create a function, castTarget. I make it take the source, target, ability ID, order string. I also create other ones for no-target and point-target if needed. Then I put those 5 lines into that function.

Now when I use a dummy caster, it is 1 line of code.

Here's my opinion on refactoring. If you're copy pasting lines over 3-4 length, you should refactor it into a function if possible. I know 3-4 lines does not sound like a lot, but the point is that this really adds up. You'd be surprised how much more readable code is when you can easily read the logic behind it, as you can when each function is kept short with well named method calls.

 

[Dr Super Good]

Here's my opinion on refactoring. If you're copy pasting lines over 3-4 length, you should refactor it into a function if possible. I know 3-4 lines does not sound like a lot, but the point is that this really adds up. You'd be surprised how much more readable code is when you can easily read the logic behind it, as you can when each function is kept short with well named method calls.

The same situation applies with procedural coupling of function arguments. In your example if you are calling the "castTarget" function with mostly the same arguments in different locations, eg the same ability and order, one can wrap it in another function to remove arguments for those 2 parameters. This way if either were to be changed one just has to change them in that function, rather than every occurrence of those arguments.