Event declarations and firing Events

[Charles]

Oh, and responses to your earlier posts:

-- When referring to nil arguments, I meant when the event handler signature is "def handler(sender as Object, args as EventArgs)" and the coder says "raise .someEvent". I want Cobra to put in "this" and "EventArgs()". I think you're saying you wouldn't bother with the EventArgs argument in that case, but you might inherit the event, or use an existing signature/delegate or may wish to follow the .NET pattern for event handlers. Or you're saying it's a bad practice, but I'm pretty use to "default arguments" from Python and get along with them fine.

-- Regarding protected events, I wasn't even thinking about them because the whole point of the event is to communicate with the outside world. In fact, for something protected, it's easier to just have a protected method that you invoke (ex: _handleReset). Nevertheless, I'll accept the patch that covers this case.

-- Nice catch on the parser.

[Charles]

Patch applied. Thanks.

[hopscc]

Lets see;
No didnt see the error/race cond fire without the local variable - would have to have a MThd access and just hit it right.
Just noticed that a number of the MSoft doc pages (from .Net2.0 or 3.0) around
event raising have the newer pattern and a big comment about doing it otherwise being a race ( mentioned .Net1.0 using the older pattern and discovery that its a race condition)
- also its analogous to similar situation with check and use in Java

nil args:
yep - Pure event - wouldnt bother with args
If you inherit the event then shouldnt you defer to the existing event raising base ( on<Name>Event) rather than raise the event yourself - how do you know that
raising that event is all the existing event raising base method is doing...
Use an existing sig ( that has args, but they're goanna be unused) - OK encourage more obfuscation
If the .Net pattern for event handlers passes an arg expecting it to be unused for (pure event )Notification thats probably an anti-pattern...
I'm not against default arguments just that for state info (communicating to otherwise decoupled items) I think you should have to present explicitly (in code whoever follows you
can read) what that info should be.///

protected events
OK - good

[hopscc]

WTF?
I updated the cobc src last night and rebuilt my apps
This warning was a nasty surprise - hitherto silently compiling code that is as I want it is now bitching at me

Code: Select all

events.cobra(35): warning: Unnecessary "this" which is already implied by raising an event. You can remove it.


(but I've slept on it and I'm calmer now)

Reasons for wanting a fully specified 'raise' without nagging
- A really really strong belief in the necessity/clarity and utility of allowing explicit specification
of all the eventHandler supplied args. Unfortunately I've been burnt by errors in defaulting
elided event args in the past and it works for me (and it seems anyone who follows on from me)
to have them explicit.
I neither need nor want nagging that I can elide them nor additional (spurious)
warnings about syntactic details to wade through vs real ones that may actually indicate problem areas.

- Allowing visual matching of arglists in all 3 components of event use
(eventHandler, sig/delegate, event raise/triggering)

- Similarity/Leverage any familiarity with the underlying C# construct
(and all the avail .Net doc that describes events and event raising...
and for all I know any/all other .Net languages - Boo, IronPython,..)
...
The warning assumes/mandates that the normal cobra construct is 'raise Eventname, eventArg'
(its probably goanna be the most commonly used) and anything else must be whined about

I think its at least as, if not more preferable/desirable to have (or at least silently allow ) the canonical
construct to be 'raise Eventname, source, eventArg' and allow 'raise Eventname, eventArg'

Assuming its likely to be most common behaviour and allowing it ('this') to to be implied is fine
nagging about having it explicitly is something else entirely

So,
can we lose this warning for a matching param count arglist ?

Have it emitted only if already defaulting some args (raise .eventName, this)
( and/or fix it in Documentation (:-) - Note that the elided this form is probably most common use
and that its available)

Otherwise:
provide specific warning suppression - more verbiage (:-(
specify another class of (labelled?) syntactic documentation 'warnings' - syntaxOptimisationSuggestions?? that can be easily shut down
???

[Charles]

I think that "WTF?" was too strong for a feature that had only been in Subversion for 2-3 days. But you said you're calmer so onwards:

I hear your reasons for wanting an explicit "this". The one about matching arg lists was the strongest in my opinion.

Btw Cobra is quiet about "raise .eventName, EventArgs()" even though you could simply write "raise .eventName"... so it doesn't whine about "anything else", only the explicit "this".

Here is what influenced me:

-- Cobra already leans towards "semi-normalized syntax". For examples, it warns about unnecessary calling parens ("foo.bar()"), unnecessary top parens ("if (x == 5)") and unnecessary this ("this.foo"). This is a theme that will continue because I believe it has value. The only other warning I know of that's not yet implemented is "if foo.someBool == true" which I see all the time in C# code. Well, perhaps also "assert foo, this" as "this" is always included in an assert/require/ensure failure.

-- In .NET, the "this" in an event raise is amazingly redundant. You can only raise your own events. In theory, you could provide a different object. In practice, I haven't seen it happen once on a real project. I think it's almost as bad as "someInt+0" or "someNumber*1". Cobra does allow the explicit arg if you need to pass something else.

-- I don't perceive any danger in an implicit "this".

-- "raise" is a special statement and therefore I don't find it strange that it has special provisions (protecting against race conditions and null pointers) and special syntax. Those aspects are the point of having the special statement.

Regarding warning suppression, I think that would be a fine feature, but it's also at the bottom of my list. A patch would be accepted.

[hopscc]

No argument about general points allowing seminormalised syntax, no danger to implicit this , raise 'special' syntax/provisions but dont see that any of this should throw warnings in your face
for raise specifically - to allow explicit specification of all args to present to decoupled receiver - you dont have to use it but you can if you think you need to (without nagging)

Maybe sender arg on raise (for arg passed to event handlers) is mostly redundant but not always - in practice (real projects) I have seen/ had to use it ( couple situations in 2 different languages) which is why I wanted it allowed .. whatever , thats not this particular point

I guess what you're saying is that no you dont want to lose the warning
that being so warning suppression ( for this at least) you will get.

[Charles]

Correct, I don't want to lose the warning. Also, just to be clear, the explicit non-this case works fine without warning:

raise .myEvent, fakeSender, EventArgs()

So when it's necessary to fake the sender, you can do so.

[hopscc]

I think theres a bug in the eventArgs Defaulting ( or alternatively a bug in the description for it).

Also, if the events argument has a default constructor, it too can be omitted,

On my system the following program gives a compile error for the elided no-args event constructor even though one is there.

Code: Select all

# raise example showing defaulting bug.
# - on my system (clean tree) if compile as is compilation fails on
# line#40 below (tagged <--).
# uncomment commented lines 'def init' and  'ev = BEvent()' and comment
# succeeding to get BEvent with only a default constructor compiles/runs OK

# compiler lookup for init in BEventArgs is only finding/using single init method and its the wrong one...
# Changing decl order inits in BEventArgs doesnt seem to help

class BEventArgs
   inherits EventArgs
   var _value as int
   get value from var
   
   #def init   # single (noArg) constructor - compiles OK
   #   base.init()
   #   _value = -1

   # 2 ctors, noArg and 1 arg - compile fails below
   def init   #noArg ctor to use raise with defaulted args - but not found
      .init(-1)   
      
   def init(val as int)
      base.init()
      _value = val
   
class EventSource
   var _count as int
      
   sig BEventHandler(sender as Object, args as BEventArgs)
   
   event bEvent as BEventHandler

   def _onBEvent(args as BEventArgs)      # fireBEvent
      raise .bEvent      # <-- default the args - this, BEventArgs()
      #^ BUG fails to find BEventArgs.init()
      #  gives error: Missing argument for "raise" of type "BEventArgs".
      #raise .bEvent, BEventArgs()         # this works
      #raise .bEvent, this, BEventArgs()   # as does this except for warning

   def bump
      _count += 1   
      if _count % 20 == 0
         #ev = BEventArgs()  # for single constructor case
         ev = BEventArgs(_count)
         _onBEvent(ev)
         
         
class Consumer

   def doBump(source as Object, eArgs as BEventArgs)
      print "Bump from [source] count=[eArgs.value]"

   def main is shared
      es = EventSource()
      c = Consumer()
      listen es.bEvent, ref c.doBump 
      
      for i in 100
         es.bump


[Charles]

I'll take a look. Might be a failure to handle overloading.

[Charles]

I've applied your patch for the "no warnings" on a given line so you can suppress them in specific circumstances.