about a patch

[Thierry Bastian]

Ok so let's say we go for Empty by default, which I agree makes sense.

Then you can:
mediaObject->setCurrentSource(MediaSource());

or 

mediaObject->setCurrentSource(MediaSource::Empty);

I still don't see the need for 

mediaObject->setCurrentSource(MediaSource::empty());

Any comments are welcome.

Thierry

-----Original Message-----
From: Matthias Kretz [mailto:kretz at kde.org] 
Sent: mardi 25 novembre 2008 14:02
To: phonon-backends at kde.org
Subject: Re: about a patch

On Tuesday 25 November 2008 13:47:15 Thierry Bastian wrote:
> I don't like too much the MediaSource::empty(). It seems to be just there
> to replace the constructor which is already there.
> Is it really more convenient to write
>
> MediaSource ms = MediaSource::empty();
>
> Over
>
> MediaSource ms(MediaSource::Empty);
>
> I prefer the latter.

Yes, but the problem is that the ctor then allows you to put any 
MediaSource::Type there, and only Invalid or Empty make sense. Also most of 
the time you'll be doing

mediaObject->setCurrentSource(MediaSource::empty());
or
mediaObject->enqueue(MediaSource::empty());

anyway.

> Apart from that I'm also wondering what is the difference then between
> Empty and Invalid... Maybe it is the same as what we have for QVariant for
> example where we have the concept of isNull and isValid. Maybe Empty
should
> then be called Null ?

Actually 'MediaSource::empty().isValid() == true' but
'MediaSource().isValid() 
== false'. (The isValid method doesn't actually exist, but that's how I'd 
implement it if it were added.)

Invalid was used for a state where something went wrong. Like passing a non-
existent filepath or invalid URL or such. Empty OTOH has the function to
empty 
the MediaObject, i.e. put it in a state where it cannot play unless you tell

it a valid, non-empty MediaSource.

> And should the default still be invalid? Actually I think so (like in
> QVariant).

So far MediaObject defaulted to Invalid because that was the only
possibility. 
Now with Empty and its meaning to put the MediaObject into a state where it 
doesn't have a MediaSource to play I think it should start out with Empty, 
since this new state better describes the initial state.

> Other than that, I'm fine with the patch.
>
> Thierry
>
> -----Original Message-----
> From: Matthias Kretz [mailto:kretz at kde.org]
> Sent: mardi 25 novembre 2008 13:40
> To: phonon-backends at kde.org
> Subject: Re: about a patch
>
> On Monday 24 November 2008 12:10:37 Matthias Kretz wrote:
> > As I said, I'm looking into making phonon-xine work without the
> > setNextSource call. I'm confident this is going to work, so we could
> > revert the revert in trunk then.
>
> Phonon-Xine in trunk can now act correctly without the call to
> setNextSource(MediaSource()). I'm fine with removing it in trunk now.
>
> > But I also had the idea of introducing Empty or Unset to
>
> MediaSource::Type.
>
> > Then Invalid would keep its meaning of doing something wrong in the
> > application while Empty/Unset would tell Phonon to actually do
something:
> > unset the current source. What do you think?
>
> I have a patch attached. Please review.
>
> --
> ________________________________________________________
> Matthias Kretz (Germany)                            <><
> http://Vir.homelinux.org/
>
>
> _______________________________________________
> Phonon-backends mailing list
> Phonon-backends at kde.org
> https://mail.kde.org/mailman/listinfo/phonon-backends

-- 
________________________________________________________
Matthias Kretz (Germany)                            <><
http://Vir.homelinux.org/

_______________________________________________
Phonon-backends mailing list
Phonon-backends at kde.org
https://mail.kde.org/mailman/listinfo/phonon-backends