Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rationalise some inheritence #178

Closed
EssyGreen opened this issue Jun 28, 2012 · 10 comments
Closed

Rationalise some inheritence #178

EssyGreen opened this issue Jun 28, 2012 · 10 comments

Comments

@EssyGreen
Copy link

The Conclusion object's only unique feature is the collection of source references it has . A collection of source references is also contained within the Person and Relationship objects so shouldn't the Person and Relationship inherit from the Conclusion? This also makes sense from a logical perspective since a Person/Relationship is a "Conclusion" (=Hypothesis/Theory in my terminology).

If so, then it would also make sense for the Conclusion object to contain the collection of Note references (moving them from the Person and Relationship objects), hence enabling Notes for things like Facts, Names, Gender etc

@EssyGreen
Copy link
Author

Conversely the Note object currently extends the Genealogical Resource (which has the Attribution in it). I can't see how the Attribution can be relevant without the bundle of source references ... Isn't that like saying something and then saying you only "Possibly" said it!? If the Note inherits from the Conclusion instead of the GenealogicalResource then it makes sense since the Attribution is related to the interpretation of the Sources expressed in the Note.

If the Note inherits from the Conclusion (along with the Person and Relationship as above) then the Conclusion and GenealogicalResource become one and the same thing. This is logical since one person's "Conclusion" is another man's source.

@jralls
Copy link
Contributor

jralls commented Jun 28, 2012

I think that the Person and Relationship classes aren't subclasses of Conclusion because of the "The sources of a conclusion MUST also be sources of the conclusion's containing entity (i.e. Person or Relationship )" constraint, which would otherwise be circular.

But it's an odd constraint.

Anyway, isn't this part of #134?

@EssyGreen
Copy link
Author

Not sure I follow you ... if Person A has a Birth event say and that Birth event cites source S1 then it follows that S1 must also be in the overall source collection for Person A along with the proof statement which explains why Person A is the same guys as the person in S1. I don't see how that is circular

I didn't put it in #134 'cos that's entitled "Shared Events" and this ain't that :)

@jralls
Copy link
Contributor

jralls commented Jun 28, 2012

You're looking at it with your genealogist hat on. Put your programmer hat on and consider (hope you can read C++):

 class A;

typedef std::set <A> aset;

 class B {
   aset foo;
 }

 class C: public B
 {
 public:
   bool is_subset(aset);
 }

 B::B (aset bar, C *container)
 {
   if (container && !container->is_subset(bar))
     throw std::exception("Not a subset");
 }

See how it's circular?

#134 started off as Relationships are N-Squared: Shared Events, but it's long since been a general discussion of the class structure and mapping to the GPS. #141 is another long discussion that covers some of this ground.

@EssyGreen
Copy link
Author

Now you've really lost me! Try java? or even plain old C? or ... here's a thought - English ;)

I realise about the other posts but wanted to highlight this separately - those other threads are already pretty stuffed with other complexities. If it's really inappropriate I'm sure Ryan will close me down :)

@jralls
Copy link
Contributor

jralls commented Jun 29, 2012

Well, hmm. My Java is a bit weak, but let's try.

public class A;

public class B {
   private List <A> foo;
   private C container;
   public void insert_A (A bar) {
      if (this.container.has_A (bar)) {
        this.foo.add (bar)
      }
}

public class C extends B {
  public boolean has_A (A bar) {
    return self.foo.contains (bar);
  }
}

Doesn't really look too different from the C++, but that's probably because I can think in C++ and not in Java.

Anyway, the issue here is not conceptual, it's implementation. You're asking for Person (class C) to be a subclass of Conclusion (class B), but Conclusion has a constraint that a SourceReference (class A) in its source list be also present in a containing Person object.

@EssyGreen
Copy link
Author

Ah gotcha! I see what you mean ... in that case I would tend to suggest that we ditch that constraint - the application can always collate all sub-citations at the person level if necessary anyway.

@jralls
Copy link
Contributor

jralls commented Jun 29, 2012

Yes, that's why I said it was a bit weird.

@EssyGreen
Copy link
Author

OK - so what do you suggest we do about it?

@jralls
Copy link
Contributor

jralls commented Jun 30, 2012

Well, for implementing your proposed change I'd remove the constraint. But ISTM that there's enough open in #134 (including Ryan's Event class) that it's premature to worry about tweaking the class hierarchy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants