weiShen weiShen - 1 year ago 36
Java Question

Why another branch is unreachable in my code?

Why the output of the following code is always

suck
. How to get
happy
as the output? Why the
happy
branch is unreachable?

public class HowToMakeStackoverflowBetter {

private static final int HUMAN_PATIENCE = 10;
private List<Member> members = new ArrayList<>();
private int atmosphere = -10;
private Random r = new Random();
public HowToMakeStackoverflowBetter(int size) {
for (int i = 0; i < size; i++) { members.add(new Member()); }
}
public Member pick() { return members.get(r.nextInt(members.size())); }

public class Member {
private int patience = HUMAN_PATIENCE;
private Question question = null;
public Member() { patience = r.nextInt(patience+1) + atmosphere; }
public void vote(Question q) {
if (patience >= 0) {
voteUp(q);
} else {
voteDown(q);
}
}
public void ask() {
question = new Question();
for (Member member : members) {
member.vote(question);
}
}
private void voteUp(Question q) { ++q.vote; }
private void voteDown(Question q) { --q.vote; }
public String toString() {
return (question.vote >= 0)? "Happy!" : "Suck!";
}
}

public class Question { private int vote; }

public static void main(String[] args) {
HowToMakeStackoverflowBetter stackoverflow = new HowToMakeStackoverflowBetter(100);
Member me = stackoverflow.pick();
me.ask();
System.out.println(me);
}
}


After a 1000 times loop, it gives us 1000 sucks. I remember 2 or 3 years ago, this was not the case. Something changed.

Answer Source

Two problems. First:

linkedList::linkedList(){  
    *sentinel.last=sentinel;  
    *sentinel.next=sentinel;  
    sentinel.str="I am sentinel!!";  
}; 

sentinel is your member variable, and .last is its pointer to another node. This hasn't been initialised, so trying to use it is undefined behaviour. In practice, it's effectively pointing at a random address in (or out of) memory, and you attempt to dereference the pointer then copy the entire sentinel object over the node at the imagined pointed-to address: i.e. you try to copy the 3 pointers in the sentinel node member variable to a random address in memory.

You probably want to do this:

linkedList::linkedList()
{  
    sentinel.last = &sentinel;  
    sentinel.next = &sentinel;  
    sentinel.str = "I am sentinel!!";  
}

Secondly, you explicitly call the destructor for linkedList, which results in undefined behaviour when the compiler-arranged destruction is performed as the object leaves the stack scope it's created in - i.e. at the end of main().


I suggest you change node.str to be a std::string, as in any realistic program you'll want to be able to handle variable text, and not just point to (constant) string literals. As is, if you mix string literals and free-store allocated character arrays, you'll have trouble knowing when to call delete[] to release the memory. You could resolve this by always making a new copy of the string data to be stored with new[], but it's safer and easier to use std::string.

Recommended from our users: Dynamic Network Monitoring from WhatsUp Gold from IPSwitch. Free Download