Hey Ryan and Patrick, thanks! I've collated some of the notes and tried 
to comment in-line.

On 04/26/17 16:39, Ryan Davis wrote:

> # 0) Delete all trailing whitespace. Use your editor to help you code like a pro.

Just started using rubocop to help find places to clean up. My editor is 
vim.  ;)

> # 1) upcase & downcase is showing that you°«re not normalizing data
> #    when it comes in / gets stored. I haven°«t looked at your code,
> #    but chances are all this data is internal and doesn°«t need ANY
> #    up/down case.

Hmm...you're right. I have morphed how this class/module is used over 
time and haven't set an internal state expectation. The methods in 
CharacterTools used to be in Character itself, then I started trying to 
clean things up a bit. Issue #45.
https://github.com/LeamHall/CT_Character_Generator/issues/45


> # 2) nobility doesn't change. Pull it out and make it a constant:

Issue #44.   :)
https://github.com/LeamHall/CT_Character_Generator/issues/44

> # 6) Which shows one real problem, this is called `title` but really
> #    sets it and returns it, every time. Either rename it or just
> #    return and set it somewhere else.

The Social Status characteristic can change over the course of an 
instance's usage. That's why "title" is calculated each time, not set.

> # 7) The other real problem is that this is a class method on a module
> #    that is acting on Character. This isn't OO. This is procedural.
> #    It is the responsibility of a Character to know its title and
> #    shouldn't be externalized.

I had to go re-read to make sure I understood this; I'm very much a 
shell programmer trying to learn OOP and Ruby.  :)


On 04/26/17 15:15, Patrick Bayford wrote:
 > Had a good look at your classes, and I can't see any obvious way of
 > simplifying or improving them. However, please bear in mind that I am
 > a relative newbie to Ruby too, although my primary interest is in
 > RPG/Combat games. - maybe one of our members more skilled at
 > re-factoring may be able to suggest a radical approach from
 > an outsiders viewpoint.

Well, this is for an RPG, so feel free to jump in!

 > My main concern is that your classes are quite large, and may
 > prove difficult to maintain/document.

You should have seen them before!  :)

The Module CharacterTools and the Class Career are pretty large as they 
contain the methods used to modify the Character instances. The grouping 
is fairly logical, though large. It comes from the Traveller (tm) RPG


 > Just as an aside, have you considered yet how to store and recover the
 > data?

Several times! Early on the plan was JSON. However, as the need has 
grown data will be stored in SQLite, JSON (to go to MongoDB), and 
something else so things can be put into Neo4J. At the moment I'm trying 
to reduce the number of gems required to run the program and removing 
the JSON requirement. Next is SQLite; that's used to access the Names 
database. I'll probably wind up with YAML to stay with the Ruby Standard 
Library and to stay simple.


Part of the reason for separation between the Character class and the 
CharacterTools module was to simplify things and clean out cruft from 
organic growth. The Character class mostly just stores data and will be 
used when putting stuff into a database or pulling it out. 
CharacterTools has the methods to generate and modify the data of a 
Character instance. The use of class methods is mostly me still trying 
to figure things out.

For example, the Presenter class will be the way instances of Character 
are displayed. Eventually there will be at least one class that will 
fetch data from a datastore and pass it to Presenter or something else.

The original concepts came from my PHP code, Since I started learning 
Ruby this seems a good project to learn on.


Leam


Unsubscribe: <mailto:ruby-talk-request / ruby-lang.org?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-talk>