Prolog First Steps: What is wrong about this code?

Question

I just started learning Prolog and can not find my mistake. When I want to find sisters (and parents) of jana with sister(jana) Prolog just returns true.

child(hanna,sabine).
child(hanna,peter).
child(robert,sabine).
child(robert,peter).
child(peter,jana).
child(peter,christine).
child(claudia,jana).
child(claudia,christine).
child(claudia,jutta).
child(jana,jakob).

female(claudia).
female(jana).
female(christine).
female(jutta).
female(sabine).
female(hanna).

male(peter).
male(robert).
male(jakob).

sister(Person) :-
   female(Sister),
   male(Father),
   child(Father,Person),
   child(Father,Sister),
   female(Mother),
   child(Mother, Person),
   child(Mother, Sister).

Answer 1

This is syntactically correct - returns true when Person fulfills the criteria e.g. has a female sibling, aka a sister.

sister(Person) :-
   female(Sister),
   male(Father),
   child(Father,Person),
   child(Father,Sister),
   female(Mother),
   child(Mother, Person),
   child(Mother, Sister).

You could use this to get all people who have a sister by doing this:

?- sister(X). 
X = jana ; 
X = christine ; 
X = jana ; 
X = christine ; 
X = sabine ; 
X = peter ; 
false.

As you can see, a person can have more than one sister, also, actually as the predicate doesn't check if Person ans Sister are the same or not - every girl is considered to be her own sister (see the answer by @mat to solve this). And also, which didn't occur to me, guys can have sisters too...

_{Result provided by @Lester, so this is the real output.}

To get what you want, you need to define it this way:

sisterOf(Person, Sister) :-
   female(Sister),
   male(Father),
   child(Father,Person),
   child(Father,Sister),
   female(Mother),
   child(Mother, Person),
   child(Mother, Sister).

And run it like this:

?- sisterOf(jana,X).
X = jana ;
X = christine.

This seems weird, isn't it? But again, as we didn't check if Sister and Person are the same or not, every female is considered to be her own sister by the predicate.

_{Disclaimer: I have learnt prolog ~10 years ago and never used it, so I'm definitely not up to date. Also, I don't have a Prolog engine anywhere near I could access, so the initial results were the result of the Prolog interpreter hosted in my brain, and updated the answer with the real results from the comments.}

Answer 2

Such problems are common among beginners and are often easily solved by using better predicate names.

Importantly, note that a predicate defines a relation between things. Between which things? Ideally, this is reflected in the name of the predicate, denoting what each argument stands for.

For example, your code becomes a lot more readable if you write it as follows:

sister_person(Sister, Person) :-
   female(Sister),
   male(Father),
   parent_child(Father, Person),
   parent_child(Father, Sister),
   female(Mother),
   parent_child(Mother, Person),
   parent_child(Mother, Sister).

Note that I am using for example parent_child/2 to make clear which argument is the parent, and which argument is the child.

So, it is clear that a predicate like sister/1 can only state something about a single person, whereas you need a relation between two people. Hence, I have called it sister_person/2.

And so it works:

?- sister_person(jana, S).
S = jana ;
S = christine ;
false.

You can use dif/2, denoting disequality of terms, to state that "nobody is their own sister":

sister_person(Sister, Person) :-
   dif(Sister, Person),
   female(Sister),
   male(Father),
   parent_child(Father, Person),
   parent_child(Father, Sister),
   female(Mother),
   parent_child(Mother, Person),
   parent_child(Mother, Sister).

Now you get a single solution:

?- sister_person(jana, P).
P = christine ;
false.