TimedLoop

[akolyt0r]

Originally Posted by Strilanc

You should return an enum (an integer, with some public constants). Nobody disagrees about what RET_REMOVE and RET_KEEP mean.

You could just keep what you have so far, and add two globals as strilanc suggested ...but IMO they dont have to be be booleans, just 2 extra constant booleans for those who arent sure about that thing ...

[Vexorian]

they dont have to be be booleans,just 2 extra constant booleans

huh.

I think the question is "keep running the loop?" and not "should I destroy it?" at least in my case it makes more sense to use true = positive = continue and false = bad = stop , true = destroy sounds a little counter-intuitive, and well... it is not like changing it would make it easier to move from another system to this one. So, I'll keep it with true = continue.

[Pyrogasm]

I personally like the true -> destroy method better, but I think in the end it's all just a matter of opinion. I do not believe that either way is particularly better/more standardized.

However, I do agree with Strilanc that it would be smart to have some constant globals inside the library to use:

JASS:

globals
    constant boolean RET_KEEP = true
    constant boolean RET_REMOVE = false
endglobals

That way, what should be done is never ambiguous and JASSHelper will inline the variable anyway.


Aside from that, I don't see any flaws in this module, really. You did write "u" where you should have written ".u" in your example, however.

[akolyt0r]

damn my last post seems obfuscated even to me xD
well basically i wanted the same pyro has just posted, with a minor twist.

JASS:

globals
    public constant boolean CONTINUE = true
    public constant boolean REMOVE = false
endglobals

[Strilanc]

Originally Posted by Vexorian

I think the question is "keep running the loop?" and not "should I destroy it?"

You think. You can't expect people using the system to think like you, and you have to design with that in mind. Enums solve the problem, using true/false means avoidable one-time bugs.

[Captain Griffen]

Given that code outside the library has to use the correct boolean, that's a terrible idea to have two constants.

[grim001]

It is sad to see this thread filled with pages of quibbling over this minor detail. Anyway, I like it the way it is, as it's the same way I do things with my periodic events. No globals.

[Pyrogasm]

Originally Posted by Captain_Griffen

Given that code outside the library has to use the correct boolean, that's a terrible idea to have two constants.

Only if the booleans were private...

I see no reason not to have constants.

[Vexorian]

You think. You can't expect people using the system to think like you,

Yes, I can. If they don't then they will find out by the time they read the documentation.

enums would be an absurdly retarded thing to do .

I am still not updating this because I can't find good names already, with the lame, completely unnecessary idea of using enums, it would get even worse, for correct scopeness of the constants I would have to add a prefix

JASS:

return PeriodicModule_Continue

it sounds terrible. Are you returning the function, or continuing the loop or returning a random constant?

I find it very irritating that every single piece of code using a boolean return value for continue or not has been able to do it, and now just because of a lame cohadar rant you are saying that I am supposed to uglysize it by adding two constants.

[Anitarf]

No, you're not supposed to. Like you said, documentation exists for a reason. To me, the current setup makes more intuitive sense than the opposite.

[Captain Griffen]

Discussion over, you don't need to change it. It's already taken up far too much talking, so I'm making a call on it. (Plus Vex and Ani agree.) Sorry Pyro, but, really, constants would be fuck ugly.

[Anitarf]

Originally Posted by Vexorian

Names and implementation are likely to change before approval.

So, will there be any changes or are you happy with the current setup? It seems alright to me.

Perhaps you could add some @@ highlights to the usage example, making the implement line, the startLoop call and the onLoop method name stand out more? Not that the example isn't simple enough to understand as it is, but a bit of highlights can only make it better.

Is there a reason why the indentation is one tab further in the startLoop method?

[Deaod]

JASS:

library PeriodicStruct
//********************************************************
//*
//*   A library + module that are meant to make those
//* array + timer loops easy, yet still faster than
//* other alternatives meant to be easy (In other words
//* no TriggerEvaluate is involved).
//*
//* The OOPness is interesting.
//*
//*  Before implementing PeriodicLoopModule,
//* your struct needs an onLoop method that takes
//* nothing and returns boolean, if the method
//* returns false, the instance will get removed
//* from the loop and destroyed.
//*
//* After implementing PeriodicLoopModule, you can call the
//* startLoop method to add the periodic event to that
//* instance, only call it once per instance.
//*
//* I recommend to call implement just bellow the declaration
//* of the onLoop method, else it will actually use
//* TriggerEvaluate, which is lame.
//*
//*  If you feel the need to destroy the struct outside
//* the loop, well, you'll have to add a flag to it so
//* you send a message to onLoop to make it return false.
//* A more complicated module to allow that easily would
//* come later.
//*
//********************************************************


//========================================================
    globals
        public real PERIOD = 0.025
    endglobals
   
   
    //===========================  
    module PeriodicStruct
        private static thistype array V
        private static integer        N = 0
        private static timer          T = CreateTimer()
        
        private static method onExpire takes nothing returns nothing
        local integer  n    = 0
        local thistype this
        local integer i=0
        
            loop
                exitwhen (i>=.N)
                set this = .V[i]
                if not this.onLoop() then
                    call this.destroy()
                    set .N=.N-1
                    set .V[i]=.V[.N]
                    set i=i-1
                endif
                set i=i+1
            endloop
            if (n== 0) then
                call PauseTimer(.T)
            endif
        endmethod
        
        method startLoop takes nothing returns nothing
            set .V[.N] = this
            set .N=.N + 1
            if (.N == 1) then
                call TimerStart(.T, PERIOD, true, function thistype.onExpire)
            endif
        endmethod
    
    endmodule


endlibrary

This is what I'd do.

Edit: why's the "not" keyword not highlighted?

[Vexorian]

So, will there be any changes or are you happy with the current setup?

I am all but happy with the current names.

Will think about it tomorrow.

[Vexorian]

Ok, updated, finally decided on good news, and well I did the constant non-sense, I don't plan to change the values of true/false so people are free to use them directly, I know I will...

As for the names, I decided against the Module sufix, everyone is free to follow a naming convention that makes more sense, as for me, I never really added a "Struct" sufix to my structs so I don't think it is necessary to do that with Module.