Code review comment for lp:~samuel-buffet/entertainer/reactive_menus

Hi Matt,

> image_menu.py:
> * I need to raise a point that will be contentious. I have to disagree with
> putting specific logic for videos/albums/clips embedded into a widget. IMO,
> the only async_add method should be async_add, not async_add_*. I know that
> this puts more burden on the screen objects, but I strongly believe that
> classes should serve only the purpose that they were intended for. In the case
> of the ImageMenu, I think it should only know how to display images that are
> provided to it. I think that the easy solution would be to move the asyn
> specific method back into the screen/tab that it came from.

I have to agree with you, this was also my concern doing that. I was pretty happy to be able to add items asynchronously but definitely not with adding specific methods for specific items.

Now questions are, what to do instead, where (I'd prefer to do that in another branch if it's okay for you).

> I have a decent idea for how to keep the async_add functionality in ImageMenu,
> but keep the specific logic out. We can talk about it on IRC. It involves
> concrete factories as input parameters to async_add (if that makes any sense
> to you right now, then our conversation on IRC will be a short one).

Okay, let's talk of tha on IRC.

Samuel-

(missing tests for text and image menus are on the road.)