tag:robotlegs.tenderapp.com,2009-10-18:/discussions/questions/79-no-subjectRobotlegs: Discussion 2018-10-18T16:35:09Ztag:robotlegs.tenderapp.com,2009-10-18:Comment/11331332010-03-03T07:06:25Z2010-03-03T07:06:25ZCheck up<div><p>I know that none have time to loose but please. thx.</p></div>jaddtag:robotlegs.tenderapp.com,2009-10-18:Comment/11331332010-03-03T17:45:42Z2010-03-03T17:45:42ZCheck up<div><p>Hi Jadd,</p>
<p>I'm afraid that it takes a lot of time and energy to do a proper
code review. From first glance it looks like you're on the right
track. I did notice a couple of things though:</p>
<ol>
<li>
<p>Proxies vs Models vs Services: I don't see a service layer in
your application. You should use services to interact with external
data. Also, I see that you're got both a DataModel and a DataProxy.
It's hard to tell by looking at the files from the outside what the
difference is between the two. With Robotlegs we've got rid of the
notion of Proxies in favour of Models.</p>
</li>
<li>
<p>SlideviewMediator: the mediator seems to have too many
responsibilities. Also, it is holding state for the view
(<em>autoPlay,</em> duration, _isTimed etc). The view should
contain it's own state, and expose that to the mediator via an
API.</p>
</li>
<li>
<p>Event typing: you should use the 4th parameter of
eventMap.mapListener() to ensure that your event constants don't
clash.</p>
</li>
<li>
<p>Naming: SetDataCommand, for example, is way too generic - it
could do anything. And your bootstrap command's could be better
named too: ViewCommand could be called MapViewCommand or
InitializeViewCommand for example.</p>
</li>
</ol>
<p>That's it for now! As I said, I don't have much time to review
code at the moment, so these are just things I spotted quickly.</p></div>Shaun Smithtag:robotlegs.tenderapp.com,2009-10-18:Comment/11331332010-03-03T18:36:58Z2010-03-03T18:36:58ZCheck up<div><p>Hi Shaun,<br>
I'm sorry I don't wont to bother anyone with my (stupid things!). I
know that it's hard to have time for help others. But thank you a
lot for the reply. I'm not sure (at my stage of learning!) I can
follow your intensions properly:</p>
<p>1 - Maybe my proxie could be named loadXmlService (its that what
you mean?).<br>
2 - RL best practice reads: "Mediator is responsable for the logic"
(more or less) and i tried to put all my logic in there.<br>
3. I used it (but i will take a more in depth look at the doc).<br>
4. I will use the last one you named.</p>
<p>I'm glad to hear that i'm in right side!!<br>
May i say one thing more about this help support!? The "solutions"
part of this help support is delosated empty! Why?</p>
<p>Thank again.</p></div>jaddtag:robotlegs.tenderapp.com,2009-10-18:Comment/11331332010-03-03T19:58:51Z2010-03-03T19:58:51ZCheck up<div><p>Hi Jadd,</p>
<p>Yes, I would rename the Proxy to Service. But again,
LoadXmlService is very generic and could mean anything. A more
specific name would be better; what does the service do? It loads
Slides. So, perhaps:</p>
<p>ISlideDataService - the service contract<br>
SlideDataXmlService - an implementation that deals with XML<br>
SlideDataJsonService - an implementation that deals with JSON</p>
<p>Also, your model deals with XML internally. The purpose of the
service layer is to translate the external data (in this case: XML)
into a format that is natural to your application model. This is so
that you can swap the service (to JSON, or AMF for example) without
having to change your model. It's the services job to load AND
translate the data.</p>
<p>Also, I wouldn't pass the url to the load() method - what will
happen when you switch to another service? You will need to change
that url. Instead, I would pass the url to the service on
construction (or after construction via another public method:
setURL perhaps), and elsewhere in your app just call
service.load(). That way each service can point to a different url
and you don't need to edit the places in your app where you call
load(), but rather just the one place where you configure the
service.</p>
<p>Mediators, most of the time, should just be passing messages
between the application and the view. It's job is to translate
messages (and payloads) between the two. Sometimes it's ok to have
a bit of logic in the mediator, but usually it's better for the
view to encapsulate it's own behavior.</p>
<p>As for the "solutions" section being mostly empty: when we set
this site up we put most of those kinds of things (solutions) into
the FAQ. The solutions section is for anyone in the community to
add to. Robotlegs is still very new, so I think it will take some
time to fill up that area :)</p></div>Shaun Smithtag:robotlegs.tenderapp.com,2009-10-18:Comment/11331332010-03-10T06:55:32Z2010-03-10T06:55:32ZCheck up<div><p>HI all.<br>
Don't curse me, I'm here again with modified version of my
slideview (As3 pure project made with Flash builder) and I hope the
last one. Again if someone want to share suggestions/opinions.
Thank.</p></div>jadd