1. Should add references to parents instead of duplicating values
         (for example Site.owner_id and Site.owner_session should be a
         reference to the instantiated objects, not the values
         themselves
[vlad] i added a owner property of type User to the site class, modified the get_owner & set_owner functions to use the user type [vlad] modified the owner_session property and made it of game_session type; modified the set_owner_session and get_owner_session so that they use the game_session type as param
      2. The values stored in the session should not be directly accessed
         by the objects (it would be the equivalent of using global
         variables) - instead, the GameSession object should instantiate
         the objects with the values it retrieves from the session, so
         that the objects never need to access the session variables.
[vlad] added a __construt function to the game session class. in there i will set the values from the $_session
      3. Should pass objects and not row ids as arguments. For example,
         User.set_homebase() should receive a Site object as an argument,
         not the id of the site, which should remain internal information
         of the corresponding Site() object.
[vlad] done

      4. Same thing with some of the properties, like Site.owner_id - it
         should be a Site.owner property pointing to a User object. When
         you don't want to instantiate some of the attributes until they
         are accessed, you can have a look at overloading
         http://php.net/manual/en/language.oop5.overloading.php  or even
         setup real properties capabilities on a BasObjecte that all
         classes would inherit
         http://www.php.net/manual/en/language.oop5.properties.php#98267
[vlad] done, as per point 1 from feedback. didn't use the overloading functionality
      5. The Leaderboard probably belongs more as a child of the
         GameSession than of the User - it forces you to pass the User
         object as an argument every time, but the Leaderboard is not
         specific to each user - it's the position in the leaderboard
         which is specific to each user
[vlad] added the leaderboard property and get_leaderboard method to the game_session class and removed them from the user class [vlad] added the get_user_rank method to the user class and removed it from the leaderboard class [vlad] modified the user_stats function from the leaderboard so that it receives a parameter of type user instead a list_of_sites array
      6. Not sure how you intend to use the User.get_*() and User.set_*()
         methods - my guess is that those methods are used to retrieve
         and store the relevant data in the DB, right? The thing is that
         those values are already stored on the object, so for example
         you don't need to pass the avg_pr value as an argument to
         User.set_avg_pr() - this method can retrieve it from the object
         itself. Renaming the methods as User.push_*() and User.pull*()
         (to respectively store the value from the object on the DB, and
         update the object value from the DB).
[vlad] from what i've read is good practice to not access the properties of an object directly so i use getters and setters methods to modify or read the propertires of the object. they are not intended to interact with the DB at that point.
for DB interaction i use the insert and update functions
      7. User.total_sites, User.avg_pr, User.longest_chain and
         User.longest_chain_tag belong to the SitesCollection rather than
         directly from User - they describe the user's collection of
         sites rather than directly the user.
[vlad] moved the total_sites, avg_pr, longest_chain and longest_chain tag from the User class to the site_collection class as well as the getter and setter methods for each of them [vlad] added insert and update function to the site collection class so that i can interact with the DB
      8. The User.get_site_collection() method is redundant: it's already
         a property of the object, instantiated by the constructor.
[vlad] removed the get_site_collection function and added the constructor function that will instantate the site_cllection object
      9. User.hack_last_refill, User.hack_last_refill, Site.last_check,
         etc. should be DateTime objects rather than strings, it will be
         easier to manipulate afterwards
[vlad] done; i added the datetime class to represent the php builtin object because visio doesn't have such an object type by default. [vlad] the php version would need to be at least 5.2.0 to have support for the datetime object
     10. User.login() and User.logout() belong to GameSession more than
         User - both these actions change the user and will thus require
         to instantiate a new User object - which will have to be done
         from the GameSession object. You probably will need the
         register() method to be on the User() object though - the User
         itself would not change, it would just be updated and switch
         from an anonymous status to a registered status, and values like
         User.email or User.password would be updated.
[vlad] done; also moved the is_logged_in fnction to the game session class
     11. Missing description and relationship for the PlayerStats and
         TagList classes
[vlad] done. the player stats refer to the user's site collection so i linked the classs to the site_collection class which in turn is called by the user class
     12. Should probably remove the notion of tree (Site.tree_id) and
         rework the tables accordingly.
[vlad] we will have a sites table a sites collection table and a site_to_collection_association table. there will be a 1 to 1 assoc between the user table and sites_collection table; a user can have one and only one sites collection. the sites_collection table will hold data about total_sites, avg_pr, chain of tags etc there will be a 1 to many assoc between the sites_collection and the site_to_collection_association. a sites_collection can hold zero or more sites. there will be a 1 to 0 assoc between the sites table and the site_to_collection_association table. a site can belong to zero or only one collection.
     13. The Site.__construct() method should not use numerical id when
         it's instantiated by the GameSession - it should receive the
         information always accessible from the request, to avoid having
         Site logic outside of the Site class, just to get the id. When
         the user navigates to a new site, what you get from the request
         is the new site URL, not the site_id. You should instantiate the
         current site with a URL then - it's then up to the contructor to
         find the relevant row in the table.
[vlad] done
     14. I think you should have an ActionProcessor class, instantiated
         by the GameSession, that knows of all possible type of actions
         that can be performed within the game and will call the relevant
         method on the right object depending on the action. The
         GameSession would then be responsible for instantiating the
         environment, and the ActionProcessor for modifying this
         environment according to the action the user is performing. It
         will allow to better isolate responsibilities, and will also
         make it easier to log actions, since the ActionProcessor could
         then automatically call the ActionLogger with the right
         attributes, independently of the action.
[vlad] done. i used the observer pattern. this will allow us to add new actions at a later time with ease


Attachment: uml_schema_2010.10.10.pdf
Description: Adobe PDF document

_______________________________________________
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/

Reply via email to