EMBTonkin / DAL9001

It's bigger, it's better, it is probably more likely to take over the world!
0 stars 0 forks source link

space-separated lists in XML #2

Closed StrykeSlammerII closed 8 years ago

StrykeSlammerII commented 8 years ago

So this one may come down to a personal preference, but here's my two cents:

For several things, you've got a single XML tag for a list of IDs (or other things?) separated by spaces. That makes it human-legible, but it's means you're hiding structure from the XML file; IME those sorts of lists are more like:

<children>
  <int>1</int>
  <int>14</int>
  <int>272</int>
</children>

rather than <children>1 14 272</children>

That's why I was confused by the <ancestors> and <descendants> tags; I thought each of those was a single entry instead of their own lists XD

( oh hey we have @ pings here? ^_^ ) This is related to my previous Issue but its different enough that I'm not just adding to that one ;)

(And since I wasn't clear:) The big problem I see is that having a string-separated thing for the getter/setter means that anything interacting with this class will have to parse that list before using it :(

EMBTonkin commented 8 years ago

Ooo, I see what you mean now. Space separated values are probably a remnant from back when this was a school project with it's own file parsing system for specialized data. Python is really good at string manipulation, so splitting around the spaces was easy.

It does make more sense for it to be part of the XML structure the way you suggest doing it. One reason against would be inconsistency, as in the main part of the file the color and gene are space separated values in one tag. To change that I would need to summon a conclave of the DRG council. (But I don't think that would be too hard and would actually fix the problem caused by Eye Spots being two words.)

One reason it does not matter really is that the getter and setter for the Dragon wrapper class is going to parse the space separated values into a list and take in a list, so and class interacting with the Dragon class would never need to know the values were once space separated.

Those are all my cards on the table, what do you think?

As for pings, I don't think so. I get email notifications about everything that happens because I am the owner, I think you can get those too if you subscribe, or you might get an email anyway because I am commenting on your thing.

StrykeSlammerII commented 8 years ago

I was hoping we could split the color/gene stuff in the DRG format, tbh -shifty eyes- (especially since there's already the possibility of colors and genes being multiple words)

However, Dragon.getChildren() and Dragon.setChildren(String children) currently don't parse the XML into a proper list... and I'm still not expecting a custom XML parser for some reason so I keep forgetting those methods aren't relevant to the read/write methods XD Once I get that through my head I'll be less off-balance with this project.

(yeah I got emails with your replies. Shiny! But @ pings are totally a thing here; I just won't need to use 'em till/unless we have more people on the project ^^ )

EMBTonkin commented 8 years ago

I'll set the motion on the table.

As for get Dragon.getChildren() and Dragon.setChildren(String children) WHOOPS I was on autopilot when I did that. It's supposed to take in a list and spit out a list. I'll fix that tomorrow.

So once I fix that things should make scene for you (on this front at least?)? Because then we have the dragon object that is a wrapper for the XML bits, so then when we need to save we can just save the XML object without having to update it.

StrykeSlammerII commented 8 years ago

I think I'm back on track, marking this Issue as closed--thanks!