Hi all!

ducasse <ducasse at iam.unibe.ch> wrote:
> Hi ken
> 
> I can only reply for Beeper as it was in my changeset and now is in 5707
> Please please do not change it again. we spent a lot of time discussing 
> it long time ago. And at the end it was ok for me.
> Noury already went over and I checked it at that time.
> Beeper play is to be able to use polymorphically with other sounds if I 
> remember correlcty

Yes, that is correct. Btw, I also find Beeper slightly overengineered,
more below.

> > What is the value of Beeper>>play?  Why the extra level of indirection?
> > Part of a protocol?

Yes, as I interpret it the use of play is to be able to install another
"playable" as default beep.

> > Back on the subject of Beeper>>beepPrimtive:  Why even have this 
> > method?
> >  Consider Beeper class>>beepPrimitive private and use it directly in 
> > any
> > places that the methods of Beeper need to generate a beep directly.
> 
> Yes you can do that.

I agree - I was also puzzled over the instance side #beep and
#beepPrimitive. They just muddled the concept IMHO. I think I will
remove those - #play is fine by itself I think.
 
> > My development system does not currently have sound system support.   
> > So
> > my ability to actually test this is very limitted.  I did file it into 
> > a
> > 3.7a-5707 image in which sound had been disabled.  What should
> > Beeper>>beep do?
> 
> Beeper by default when there are no sound just call the primitive so if 
> the primitive
> does not produce sound, nothing. For example this is the case where you 
> could have
> a flashing beeper.
> 
> >  I assume I should still hear a system beep.  I do not.
> >  And I know the system beep works because for some reason I hear it
> > every time SM2 downloads a new map snapshot.

As I interpreted the preference soundsEnabled - I assumed any sound
whatsoever, including beeps, should be suppressed if set to false.

Furthermore I think NoSoundSystem or DummySoundSystem seems better than
BadSoundSystem.
Also BaseSoundSystem is better than GoodSoundSystem.

The name of SoundSystem seems ok - just like WebBrowser or MailSender.
But I agree it can be confused with the other classes. Hmmm, perhaps we
could call it SoundService?

The important thing with this class is that it "reads" fine. Ok, I think
I will rename these to:

DummySoundSystem
BaseSoundSystem
SoundService

regard, Göran

PS. The reason for all methods signed by my initials is probably that I
went through them all one by one and among other things added comments.
I guess.