[vlc-devel] [Patches] Dash: mostly cosmetics

[Hugo Beauzée-Luyssen]

Hi,

On Fri, Nov 25, 2011 at 7:03 PM, Tobias Güntner <fatbull at web.de> wrote:
> Am 25.11.2011 16:30, schrieb Hugo Beauzée-Luyssen:
>>
>> I'll let you decide about "Removing EOFException"
>
> Even if you keep the EOFException, it might be a good idea to get rid of all
> exception specifications. If something unexpected is thrown/propagated,
> std::unexpected() will be called, which will terminate the program by
> default. For example, getNextChunk() is declared as throw(EOFException), but
> the std::vector<ISegment*> in there could throw std::bad_alloc. Of course
> you have to actually catch those exceptions somewhere. ;)
>

I agree, but my plan was to remove all the exception, and therefore
the exceptions specifications.
Some of the exceptions are not caught, which reduces their use IMHO.

>> +    if ( this->mpdManager == NULL || this->currentPeriod == NULL )
>> +        return NULL;
>
> IMHO you should check this in the constructor and throw
> std::invalid_argument instead.
>

Actually the mpdManager can't be NULL. I guess I'll add some checks to
handle invalid profiles (the IMPDManager is instantiated according to
the profile, in MPDManagerFactory.cpp)
If there is no period, I don't think there's a point in getting a
chunk... I'll check that too.

>> +    delete this->adaptationLogic;
>> +    delete this->mpdManager;
>
> This calls for some kind of auto_ptr. ;) The copy constructor and operator=
> of DASHManager should be disabled as well (i.e., make them private).
>

I was planning on fixing this as a second pass, using -Weffc++
Regarding the auto_ptr, why not, but I'm not that bothered by 2 deletes :p

> Regards,
> Tobias
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> http://mailman.videolan.org/listinfo/vlc-devel
>



-- 
Hugo Beauzée-Luyssen