Template/Stringstream run-time error;

[BarelyOtaku]

Calling a template function causes a run-time error of freezing

Could anyone help me debug this I've spent around 30 minutes on this and I isolated the problem to be on this function.

I basically create an object:

Data<int> object(10);
//Then I call
object.setSeq(std::string someStr); //<--- Freezes program 

However, my program just freezes and never terminates from the function setSeq...

The function looks like this:

void setSeq(std::string seq){
		std::cout << "1DEBUG error";
		std::stringstream tSeq(seq);
		std::cout << "1DEBUG error";
		for (int i = 0; i < aLength; i++){
			tSeq >> dSeq; //Used operator overloading std::stringstream >> T& seq;
			typeArray[i] = dSeq;
		}
	}

The header looks like this in case the operator overloading is confusing:

/*
 * reader.h
 *
 *  Created on: Aug 1, 2015
 */

#ifndef DATA_H_
#define DATA_H_

#include <iostream>
#include <fstream>
#include <sstream>

template<typename T>
std::stringstream& operator>>(std::stringstream&, T&);

template<typename T>
class Data{

public:
	Data(){
		aLength = 0;
		typeArray = NULL;
	}
	Data(int length) :
		aLength(length)
	{
		typeArray = new T[aLength];
	}

	~Data(){
		delete[] typeArray;
	}

	T& operator[](int index){
		if (!0 <= index || ! index < aLength)
			std::cerr << "Accessing non valid index";

		return typeArray[index];
	}

	int getLength(){
		return aLength;
	}
	void setSeq(std::string seq){
		std::cout << "1DEBUG error";
		std::stringstream tSeq(seq);
		std::cout << "1DEBUG error";
		for (int i = 0; i < aLength; i++){
			tSeq >> dSeq; //Used operator overloading std::stringstream >> T& seq;
			typeArray[i] = dSeq;
		}
	}

	friend std::stringstream& operator>> <>(std::stringstream&, T&);

private:
	int aLength;
	T* typeArray;
	T dSeq;
};
template<typename T>
std::stringstream& operator>>(std::stringstream& in, T& seq){
	in >> seq;
	return in;
}

#endif /* DATA_H_ */ 

Before anyone asks... I tried

std::stringstream tSeq;
tSeq.(seq);

Also the std::cout << "1DEBUG error" doesn't output either.

[Disch]

template<typename T>
std::stringstream& operator>>(std::stringstream& in, T& seq){
	in >> seq;
	return in;
}

This operator is calling itself over... and over... and over...and over.

You're basically doing this:

void foo()
{
    foo();
}

foo will call foo... which will call foo again... which will call it again... etc etc. It will cause a deadlock and eventual crash when you run out of stack space.

The goal of overloading the >> operator is to tell it HOW to get info from the stream. For that you'll need a more detailed body.



Also -- you don't want to use stringstream in the >> operator... you'd want to use istream:

std::istream& operator >> (std::istream& in, T& seq){

This works not only with stringstreams, but other types of input streams as well (fstreams, cin, etc).

Likewise, with the << operator, you'd want to use ostream, and not stringstream.

[BarelyOtaku]

You're a god!!
Can't believe it was that simple...

In this case would it just be better to determine the type of seq and then just avoid operator overloading overall?
I can't see a way to implement this using operator overloading

EDIT: There was no point of operator overloading, since the template will change it to the type declared

Also if the problem is in the loop, why doesn't this work? :

std::cout << "1DEBUG error";

Makes debugging my error that much harder... Q_Q

[Disch]

Also if the problem is in the loop, why doesn't this work? :

Because the stream was never flushed. Sending something to cout doesn't necessarily mean it will be printed. cout is buffered, and occasionally the buffer has to be "flushed" -- which basically means taking the data in the buffer and actually displaying it on screen.

You can force a flush with 'flush' or with 'endl':

std::cout << "1DEBUG error" << std::flush;

Makes debugging my error that much harder... Q_Q

Try using a debugger instead of littering your code with cout statements. Set a breakpoint on the line of code in question instead of putting a cout statement. When you run the program, the debugger will snap when it hits the breakpoint, allowing you to see how the program is flowing, as well as allowing you to view contents of all variables at that point in time.


- Don't have to make code changes
- Don't have to recompile
- Don't have to remember to remove those cout statements after the problem is solved
- Can view contents of all variables at runtime
- Can forcefully change contents of any variable
- Can step through the code one line at a time to see how it's running


Debuggers really should be one of the very first things you learn how to use when learning programming.

[TheIdeasMan]

Hi,

Just a 1 cent thing I noticed:

	T& operator[](int index){
		if (!0 <= index || ! index < aLength)
			std::cerr << "Accessing non valid index";

		return typeArray[index];
	}

The ! operator binds pretty tight - much higher than <= etc. At the moment it is interpreted like this :

if (!(0) <= index || !(index) < aLength)

!0 is pretty insidious because it equals in binary 1111... and !index will be bad for the same reasons, any way suffice to say that neither of those values will represent the boundaries of the array.

so I am wondering why you don't just do this:

// index is in correct range 0 is still ok
if (index < 0  ||  index > aLength) {   // always use braces - even when there is only 1 statement following 

Further, If the index is actually invalid, even though you print an error message, your code still attempts to return a value - so this will result in a segmentation fault. So you need to have some more hard-nosed way of flagging this error. I am not sure whether one can return a nullptr from here - it's expecting a reference so that might be OK. Obviously, one needs to test for that - which is a bit awkward if in the middle of a larger expression.

Then there is "Make sure it's valid before using" idea.

Another thing to try might be a static_assert which produces a compile error (with your own error message) when the statement evaluates to false. But this is a compile time thing and is probably only really useful when you test with hard coded values, so it is not really providing a run time solution.

Then there are exceptions .....

Anyway, I started out by pointing out a problem - then might have got carried away explaining it - hopefully it was useful in the end :+)

http://en.cppreference.com/w/cpp/language/operator_precedence
http://en.cppreference.com/w/cpp/language/static_assert

[Disch]

!0 is pretty insidious because it equals in binary 1111

~0 == %1111111....
!0 == 1

[TheIdeasMan]

Hi Disch :+)

I am sure you are right - it's me that is due for a little tune-up education wise!

I can see how negating 0 or 1 is really just like negating false or true .

And I have this mixed up with binary negation.

So with the OP's if statement we have, if index is 0 Array Length must be positive :

if (!0 <= index || ! index < aLength)
if (true <= false || !false < true)
if (true <= false || true < true)
if (1 <= 0 || 1 < 1)
if ( false || false )
if (false)

otherwise if index > 0, or negative :

if (!0 <= index || ! index < aLength)
if (true <= true || !(true) < true)
if (true <= true || false < true)
if (1 <= 1 || 0 < 1)
if (true || false)
if (true)

So this is bad : negative index values produce true !

Jumping Juniper Berries - did I do this correctly? If I have to think about it this much, then that is why I proposed a different if statement.

@BarelyOtaku

Sorry, I found an easily fixed problem, but then proceeded to beat it thoroughly to death - I probably didn't need to reply to Disch in such detail.

[BarelyOtaku]

@TheIdeasMan

Hey IdeasMan!
I actually noticed that error during run-time and was like wth.
I fixed it quickly by just applying parentheses.

I only did that because Eclipse has a stupid bug that gives warning if your return statement is in a condition. I knew about assert but I just didn't apply it.

I wasn't trying to negate 0 xD:
Originally the statement was:

p 0<= index q index < arraySize

So to fix the Eclipse problem mentioned earlier I simply negated my statement:

 !(p && q)
!(p) || !(q) //Demorgan's law 

I also could've just change the statements to !p OR !q but I was lazy and I assumed the ! would somehow negate my whole statement xd

[TheIdeasMan]

Hi again,

I understand you weren't trying to negate 0, and using more parentheses makes things clearer, but the problems / confusion I have with your last post are:

p is wrong : zero is a valid index - it should be index < 0

q is wrong , it should be index > arraySize greater than

p && q is wrong as well, it can't be both.

so this all leads to :

if (index < 0 || index > ArraySize) {/* error exists */}

Which is what I had before, and dead easy to understand.

Another thing to note: negation semantics are often harder to understand - If one can reverse the logic so there is no negation (or at least minimised) then that is probably much easier to understand.

I am not saying get rid of negation altogether, this obviously works while(!Quit) {}, I mean just for anything more than trivial where there is potential for confusion.

I only did that because Eclipse has a stupid bug that gives warning if your return statement is in a condition.

If the IDE syntax checking or a compile warning appears about something, then that might be a clue that one is attempting something dodgy.

Cheers

[BarelyOtaku]

p is wrong : zero is a valid index - it should be index < 0

q is wrong , it should be index > arraySize greater than

p && q is wrong as well, it can't be both.

Nonono you misunderstood the intention of my control statement.

I tried to detect if the array being accessed is valid
However, since my return statement was inside the valid condition, Eclipse gave me a warning.

Therefore I negated my statement, creating the opposite of valid condition and allowing me to put the return statement outside while putting the error inside.

Your statements index < 0 or index > arraySize are applying the negation of my statement which is what I did by just adding !().

i.e. When you say my p is wrong you're saying 0<= index isn't valid aka false. Then you say that index < 0 is correct aka true , which is just the negation of my statement so . If you check my previous post, I made it clear that the alternative of my statement was yours.

That is !p is index < 0 || !q is index > arraySize

You're thinking that my initial statement stays in the same location, but it doesn't.

[TheIdeasMan]

Nonono you misunderstood the intention of my control statement.

I'm trying to detect if the array being accessed is valid

So now I am a little confused.

At the start you had:

std::cerr << "Accessing non valid index";

All right, to avoid a long discussion (which I appreciate you probably don't have time for) , can you post the code you have now?

Also, do you have an opinion on what I was saying about negation semantics?

[BarelyOtaku]

Sure!

/*
 *  data.hpp
 *
 */

#ifndef DATA_H_
#define DATA_H_

#include <iostream>
#include <fstream>
#include <sstream>
#include <typeinfo>

//template<typename T> NOT NEEDED CAUSES LOOP EROR
//std::istream& operator>>(std::stringstream&, T&);

template<typename T>
class Data{

public:
	//friend std::istream& operator>> <>(std::stringstream&, T&); NOT NEEDED CAUSES LOOP ERROR
	Data(){
		aLength = 0;
		typeArray = NULL;
	}
	Data(int length) :
		aLength(length)
	{
		typeArray = new T[aLength];
	}

	~Data(){
		delete[] typeArray;
	}

	T& operator[](int index){
		if (!(0 <= index)|| !(index < aLength))
			std::cerr << "Accessing non valid index";

		return typeArray[index];
	}

	int getLength(){
		return aLength;
	}
	void setSeq(std::string seq){
		std::stringstream tSeq(seq);
		for (int i = 0; i < aLength; i++){
			tSeq >> dSeq;
			typeArray[i] = dSeq;
		}
	}

private:
	int aLength;
	T* typeArray;
	T dSeq;
};
/* NOT NEEDED CAUSES LOOP ERROR
template<typename T>
std::istream& operator>>(std::stringstream& in, T& seq){
	in >> seq;
	return in;
}*/

#endif /* DATA_H_ */ 

I agree wholeheartedly with your opinion on negation semantics, keeping it simple.
I only negated my statement because of the Eclipse thing.
I prefer thinking of negation as statements and simply adding NOT, the idea of switching the actual statements bothers me. Although I do see positions where doing that will make the code uglier or unsmooth.

[TheIdeasMan]

	T& operator[](int index){
		if (!(0 <= index)|| !(index < aLength))
			std::cerr << "Accessing non valid index";

		return typeArray[index];
	}

Ok, so I reiterate what I said about negation semantics, what I had is easier so that makes it better IMO.

And let me show that !(0 <= index) is not the same as (index < 0)

The main reason being that positive and negative values both return true, and you have a different relational operator .

If index is is positive or negative, the result is the same:

!(0 <= index) !(0 <= true) !(0 <= 1) !(true) false

If index is 0:

!(0 <= index) !(0 <= 0) !(true) false

With !(index < aLength), that works logically because if the first part was correct and catches a negative value, the whole expression is short circuited (the first part was true, so that is all that is evaluated in the or expression). But the first part isn't at the moment correct either.

However observe what happens if one reverses those 2 expressions:

if ( !(index < aLength) || !(0 <= index) )

The first one fails for negative values of index, as does the whole expression.

To prove all this, test your operator with 4 values: negative, 0 , positive but less than or equal to array length, bigger than array length. See how you go with your current code.

I prefer thinking of negation as statements and simply adding NOT, the idea of switching the actual statements bothers me. Although I do see positions where doing that will make the code uglier or unsmooth.

Apart from making the code uglier , it can make it wrong. In terms of the actual logic: all it is, is finding out whether index is less than 0 or greater than ArrayLength . I guess if you started out with asking whether index was not between 0 and array length, then that might lead to difficulties, if you weren't careful.

Any way it is late I my end - have to be up early for work tomorrow, so I won't be able to reply for another 20 hours.

[BarelyOtaku]

I think you have some kind of misunderstanding. I'm not just applying these negations randomly, I'm using Discrete Mathematics Proposition functions.

Let's start with the first statement:
Let's first clear up that the following conditions are test if there's error and print TRUE

If index is positive or negative, the result is the same:
[The main reason being that positive and negative values
 both return true, and you have a different relational operator .]
!(0 <= index) !(0 <= true) !(0 <= 1) !(true) FINAL RESULT: [false] <--- POSITIVE EXAMPLE.

Not true both don't yield the same.

!(0 <= index) !(0 <= -1) !( 0 <= -1) !(false) FINAL RESULT: [true] <----NEGATIVE EXAMPLE. 
RESULT IS NOT THE SAME

Next, your example of reversing the statement order:

However observe what happens if one reverses those 2 expressions:
if ( !(index < aLength) || !(0 <= index))
The first one fails for negative values of index, as does the whole expression. 
 

The point of this condition is to print TRUE if there's an error.
So in this case if the first one is negative the statement returns negative alright that shouldn't be right. HOWEVER...
Move to the right one index is negative the statement returns true. Therefore there is an error.

By the Laws of Logic commutative laws stated here:
https://en.wikibooks.org/wiki/Discrete_Mathematics/Logic#Laws_of_Logic
Nothing changes the result yields the same for p if ( !(index < aLength) || !(0 <= index) )

 And let me show that !(0 <= index) is not the same as (index < 0)

I've had this beat into my head by the endless amount of assignments and midterms I had on this. They are the same believe it or not. Because the opposite of 0<= index is 0 > index. However there is a ! which makes them the same.

I wanna make sure my point is clear that they're indeed the same:

let P be !(0 <= index)
let Q be index < 0

IF INDEX IS POSITIVE

P returns FALSE, indicating no error.
Q returns FALSE, indicating no error.

IF INDEX IS NEGATIVE

P returns true, indicating an error.
Q returns true, indicating an error.

I'm new to writing C++, but that doesn't mean my sense of logic is bad.

[TheIdeasMan]

I think you have some kind of misunderstanding. I'm not just applying these negations randomly, I'm using Discrete Mathematics Proposition functions.

But my main point is that it is over complicated. Einstein once said "Things should be simple, simple as possible, but no simpler".

Another example, if we were to talk about the area of a circle: If some one said it was equal to PI * r * r , pretty much everyone would immediately see that as being correct; But if someone else was to say that it was also equal to PI * (D * D) / 4 (D is Diameter), then there would be quite a few who have to think about that a little to verify it was correct. Which it is, but I would wager that most would prefer the first one.

The thing is, code should be easy to read - which is better: Something that is dead obvious, or something where one has to ponder and analyse 4 different scenarios to see if it right, then verify that the analysis is right ?

With regards to your use of mathematical laws, I am mystified why you started with expressions that you had for P and Q. If I could make a wry observation here: We ought to use mathematics to simplify where possible, not to unnecessarily complicate.

Once I get home (another 5.5hrs)I will write a little program to test your code and mine. That might be a another way to test the validity of either case. Even if yours is right, I still prefer mine purely because of the simplicity, and I would suggest that mine might win a vote by any number of people as to which they prefer.

[ne555]

if( not in_range(0, index, lenght) )
   //error 

> And let me show that !(0 <= index) is not the same as (index < 0)
It is.
I don't understand why you transform `index' to a boolean before the `less than' operation

[TheIdeasMan]

@ne555

Your are exactly right, and I seem to be having a boolean nightmare.

@BarelyOtaku

I apologise sincerely for all the grief, confusion & time wasting. I should have tested your code, then I could have said "Your code works, but I still like mine because it is simpler" , and left it at that.

I meant to reply much earlier, but the browser didn't send the message for some reason.

Regards

[BarelyOtaku]

Sorry I didn't get the e-mail for these replies.

I completely agree with the way you suggested, my code was simple but... the reason why I changed it was because Eclipse gave me a warning and I don't like having warnings in my face.

So assuming that no one will have to work on this assignment I did that complication of double negative.

Thanks for the help guys I couldn't have done it without all of you helping me debug :D