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
[vlad] done and commited to the repository
- Dataobject files need to be chmod 644
[vlad] done and commited to the repository
- Missing a merge from lp:hackit
[vlad] done and commited to the repository
- 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)
[vlad] i updated the update file so it should be more clear what the
stept are. right now there's still the problem of David's MDB2 issue and
the deprecated problem on hackit.cx
- Missing an update of the CHANGELOG
[vlad] done and commited to the repository
- Need to add instructions to regenerate data objects
[vlad] done (added to the install file) and commited to the repository
- Format of the class names: "OneTwoThree", not "one_two_three"
[vlad] done and commited to the repository
- 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)
[vlad] i chose to have those objects separate, because it would have
been a conflict with our getter/setter implementation for the object
properties
also, for some functions (like the changes since last session section) i
need to do some reads from the database without instatantiating full
object for the elements
- 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;
[vlad] done, but not commited yet. i need to do some more tests as well
as update the unit tests to make sure the switch doesn't cause problems.
corroborated with the movement of the prepare_address function, this
proved to be more complicated than initially thought.
in prepare_address, depending on certain conditions, i need to access
data related to the current session (the user object to be more exact),
so i had to pass the current session as an argument inside the
prepare_address function from within the site class.
- Need to remove the part "TO DO remove at end of 2.0 iteration" ;p
[vlad] done and commited to the repository :)
- 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.
[vlad] i avoided to do that for the reasons mentioned at the User and
User Object answer
- 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.
[vlad] done, but not commited yet. i need to update the unit tests and
make sure it functions as it should
- 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.
[vlad] will do first think in the morning :)
- prepare_address() should be on the Site class
[vlad] done, but with the chalenges mentioned in my previous answer.
_______________________________________________
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/