[Borges-users] refactoring

[Eric Hodel]

??????? ???????? (slepnev_v at rambler.ru) wrote:

> Hi,
> 
> I'm reading the Borges source, trying to understand what to refactor 
> where, and where to stick the unit tests. IMHO, there's room for some 
> improvement.

Excellent!

> Why does Controller have #inform and #confirm?

Because Seaside has them.

> (No one's gonna use that anyway - in any kind of production job, it's
> conceptually simpler for the developer to create a custom "confirm"
> controller, with CSS, layout and such.)

Dialogs are Components.  Note that in a case such as SushiNet, you could
use confirm or inform from the @main section of the frame, and they'll
pick up whatever CSS and layout is around them.  Several of the webtests
use #confirm and #inform, and they look alright.

For example, SushiNet::StoreItemView#add_to_cart could be implemented
like this:

def add_to_cart
  session.cart[item] = request "How many #{item.title} do you want?", "Add"
end

I really like the way this reads and how convenient it is compared to
an explicit #call.

> Why does Controller#answer check for @continuation.nil? If a 
> controller attempts to answer and there's no place to answer to (or 
> will_answer? had been set to false), this should be an error that the 
> app developer will instantly see. Let's remove this check.

You're probably right with this one.  Write some tests and prove it (see
below) :)

> Why is Controller using class << self, instead of just defining 
> self.inherited ? (#initialize_controller doesn't do much; and we could 
> remove it altogether, along with class<<self, if we add some 
> @delegate.nil? in strategic places. Funny those checks actually were 
> there until I asked to remove them some time ago. So much for my 
> consistency =)

self.inherited can probably go.  Most of the class << self stuff comes
from porting from Seaside.

> Come to think of it, why do Controller and Session have to know all 
> their subclasses at all?

Smalltalk provides more dynamicity than Ruby, so you could edit an
Application and change what session class and entry point are used by
that app.  This can probably be stripped, since it is nearly useless in
Borges.

> This isn't actually needed by the framework, except in some
> "preferences" stuff, is it? And, any application or admin tool that
> needs all subclasses can easily get them from ObjectSpace, so why keep
> it in the Borges core? The less code in the core, the better.

Ok.  There really isn't much benefit in being able to change the
entry point of an application if you have to restart Borges for a change
that could take advantage of it.

> As for preferences - let's just kill them!

They may be useful, but they're really, really ugly right now.  They
provide a nice way of tuning live apps, though.

> It would much please me to see the Borges codebase polished, and it 
> would please me even more to take part in the process.

Me too, on both accounts.

> Should I start making diffs for this stuff?

Things are pretty complex, so I have been first implementing tests for a
part of the framework I want to change first, then making changes.

This lessens the chances of breaking something accidentally, or through
incomplete understanding.  Tests also help me figure out what a method
is supposed to do, since there is a lot of interaction between the
various pieces of Borges.

So my recommended course of action is:

1) Find something you want to fix
2) Write tests (if they aren't already written)
3) Submit the tests here
4) Make changes
5) Submit the changes here

Oh yeah, and once I get you 'trained' I'll hand out a commit bit (but I
prefer to see changes before they go in, regardless).

-- 
Eric Hodel - drbrain at segment7.net - http://segment7.net
All messages signed with fingerprint:
FEC2 57F1 D465 EB15 5D6E  7C11 332A 551C 796C 9F04

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 187 bytes
Desc: not available
Url : http://rubyforge.org/pipermail/borges-users/attachments/20040430/4a840cd0/attachment.bin