Hi Vlad, I've spent some time on the code to familiarize myself with the changes. There is a lot to go through, so I haven't looked at everything in details, but I think I've got a good overview of how things are structured now.
I really like it, I think it's taking a good shape. There is still some tuning to do (there will always be some), but a good bulk of work is behind now. Congratz! Here are some comments/changes/questions - I'll let you read them, let me know what you think. === - Dataobjects auto-generated for Ajaxim: Zajaxim_friends.php & ajaxim directory => is it remnants from trying to add ajaxim in the past? If so they need to be removed - Dataobject files need to be chmod 644 - Missing a merge from lp:hackit - Same issue as David - https://bugs.launchpad.net/hackit/+bug/668243 . I've updated a few files on lp:hackit, but it seems that it's missing the SQLs for the updated database structure (see the ticket) - Missing an update of the CHANGELOG - Need to add instructions to regenerate data objects - Format of the class names: "OneTwoThree", not "one_two_three" - Why did you separate User and the users data object? You add yourself a lot of trouble - you could simply add the methods directly on the dataobject, and expose it directly, no? But maybe there is something I didn't see. (And same question about the Site and GameSession classes, actually) - Instead of individual arguments about the current site directly attached on GameSession, should attach a Site as GameSession->current_site, containing: * public $web_address; * public $encoded_address; * public $proxy_page; * public $hack_probability; * public $scan_probability; - Need to remove the part "TO DO remove at end of 2.0 iteration" ;p - You could move more code to the dataobjects themselves - basically code that only require to manipulate dataobjects themselves should be considered, especially if it could be useful to different objects. - I missed that one in the UML schema: code related to hacks and scans should be isolated on other objects, here it makes the game session class HUGE. Why didn't you put it in the action observers? You could actually isolate a good portion of the GameSession logic into smaller dedicated objects this way. - Btw, you should put all the files containing the classes for the action observers in a dedicated subdirectory of classes/ . And congratz btw for the mechanism here, it works very gracefully. - prepare_address() should be on the Site class Xavier. _______________________________________________ Farsides mailing list - [email protected] Wiki: http://farsides.com/ List: http://farsides.com/ml/ Forum: http://farsides.com/forum/ Ideas: http://farsides.com/ideas/ Chat: http://farsides.com/chat/
