StuXYZ 731 Practically a Master Poster

The problem that you seem to have with this code is understanding the pathway of the if / else construction and the way to define it. There is a "formal" way to define it, but I think that first time it is easier with an example: So here goes:

#include <iostream>
int main()
{
   int x=10;       // Define x to be 10.
   if (x>2)       // This is obviously True
     {         
        std::cout<<" This statement is called"<<std::endl;
        std::cout<<" This is also called"<<std::endl;
     }
   else
     {
        std::cout<<" ****** I am NOT printed " <<std::endl;
        x=20002;        // this does not get done
        std::cout<<"  ***** I am ALSO NOT printed "<<std::endl;
     }
}

I have given a complete program, you can run and see that only the first part is done, but not the second.

If you see there are two pair of brackets { } . After an if statement, or an else, or many other C++ statements, e.g. for(...), a pair of brackets can be written, and this defines the scope of the branch or statement.

In the case above, if you change x to be say 1. Then recompile and run, you will set that the statements after the else are carried out but not any of the statements in the first { }. You can add extra statements to either section but only one will be carried out.

There are two additional points, firstly, IF you write many C++ statements, e.g. if etc, AND do not follow …

StuXYZ 731 Practically a Master Poster

I don't like the example given. I thisk that it misses the opportunity to be a useful learning tool: I would like it modified like this to allow the user to actually see what went on:

// bad_alloc example [+ modification]
#include <iostream>
#include <new>

int main () 
{
  long int allocateSize(1);
  for(int i=0;i<100;i++)
    {
      allocateSize*=10;
      try
	{
	  std::cout<<"Before allocation :"<<allocateSize<<std::endl;
	  int* myarray= new int[allocateSize];
	  std::cout<<"After allocation :"<<allocateSize<<std::endl;
          std::cout<<"--------"<<std::endl;
	  delete [] myarray;
	}
      catch (std::bad_alloc& ba)
	{
	  std::cout << "bad_alloc caught: " << ba.what() << std::endl;
	  std::cout << "Allocation size == "<<allocateSize<<std::endl;
	  return -1;
	}
    }
  std::cout<<"This is computer has a HUGE memory system"<<std::endl;
  return 0;
}

So run the code and see if you are surpised by the observation that you see the last before message but not the after message, NOR the last message.

However, take out the return -1 and observer that you see MANY bad allocates, as the loop completes.

StuXYZ 731 Practically a Master Poster

I am not surprised this doesn't run! You have a head node called root. However,
rather than add a new copy of the memory for it, you do this:

if(root == NULL)
  {
    // THIS IS A RUNTIME ERROR:
    root->data = Person(name, number);
    root->left = NULL;
    root->right = NULL;
    current = root;
  }

Note that root is 0. BUT then you access the object pointed to by it. That isn't going to work.

What you need to do is this:

if (root == NULL )
  {
     root=new node;
     // ... etc.

However, when you do that, you have to (a) ensure that you are deleting the tree in the destructor of Book. (b) ensure that a sensible copy constructor and assignment operator are written for book. (c) ensure that Person has appropriate copy constructor and assignment operator, be careful about setting current.

Finally, you are obviously moving around non-trivial objects, e.g. Person etc. Please use references, e.g.
I think Book should look like this:

class Book
{
  public:
    Book();
    Book(const Book&);  // Non simple data object ALWAYS provide a copy constructor
    Book& operator=(const Book&);   
    virtual ~Book();

    void insert(const Person&);     // Note the change to a reference
    node* search(const std::string&);
    void removeContact(const std::string&);
private:
   node* root;
   node* current;
};

Also if Book is has a virtual destructor, so will be inherited from, does insert/search etc need to be virtual too?

StuXYZ 731 Practically a Master Poster

This is just a simple multi-file program. The big advantage of this [Particularly for big projects] is that you don't have to keep recompiling all the program in one go, just the bit that you are changing.

Although it doesn't do anything you can compile it like this:
(as individual files) [This creates a set of object files: node0.o node1.o etc]

gcc -c node0.c 
gcc -c node1.c
gcc -c node2.c 
gcc -c node3.c 
gcc -c prog3.c

Then you link like this gcc -o myprogram prog3.o node0.o node1.o node2.o node3.o which will give you a program called myprogram.

However, you can do it all in one line: gcc -o myprogram prog3.c node0.c node1.c node2.c node3.c If you have another compiler, then you have to combine all the files into one project.

Additional: Note that exit(int) which is used in your code is in stdlib.h although some compilers link to stdio.h as well. gcc is not one of them so you have to add #include <stdlib.h> to top of your prog3.c file.

StuXYZ 731 Practically a Master Poster

You unfortunately have several problems: So i am going to give you a quick summary of the problem and why I think you have made the mistake.

(i) In the constructor fraction::fraction(), you have written integral, numerator, denominator = 0; This DOES NOT do what you think, it does NOT set integral or numerator. If you wanted to do that then you should have done integral=numerator=denominator=0; . Much much better would have been to use an initializer list e.g. fraction::fraction() : integral(0),numerator(0),denominator(0) {} .
Note: Almost all modern compilers would have given you a warning about that if you had turned your warnings on. No beginner should ever compile without warnings on.

(ii) Input is not actually wrong but so ugly, surely improper fractions would be ok?
What about reading a line of input and then splitting it based on e.g. 4/5 or 3 4/5

(iii) friend fraction operator+ Never make something a friend that doesn't need to be. This just doesn't need to be a friend. It should also have this form: fraction operator+(const fraction&) const; Then it is (a) more efficient (b) much less likely to go wrong.

(iv) gcd and gcdr should be one function and not call each other backwards and forwards, or gcd_prep should call gcd (which would be the recursive function) once only [to allow the test for negative to the outside.]

(v) operator+ is written in such a way that I think you did not really …

StuXYZ 731 Practically a Master Poster

You still have a couple of design issues (bugs).

In particular look at the value n. It is suppose to be the number of data items added, but actually you set it in the constructor, why not increase it each time that you add some data, and set it to zero in the constructor.

What purpose does x have??

StuXYZ 731 Practically a Master Poster

Absolutely fine, (IMHO), obviously it depends on your needs, but what about classes that return some kind of evaluation, e.g. a class that calculates the global density from all the objects in a simulation. It knows the materials, object locations and orientations. It has only one public function that returns something and that is the calcDensity function.

StuXYZ 731 Practically a Master Poster

No: That doesn't solve your problem, you have just put your problem to a slightly higher number. e.g. what happens if dicerolls exceeds 64. Same thing.

The trouble is that I don't understand what the underlying problem is that you are trying to solve [e.g. the reason for writing this computer program]. But I can see that for this algorithm at even , you are going to need more bytes than all the atoms in the universe, at modest numbers like 300 dice rolls.

This is due to line string letterlist [table[dicerolls - 1]][dicerolls]; because table is populated with 2,4,8,16 etc...

So can you give me [us ?] a quick summary of the problem that you are trying to solve please, and hopefully, some of the community here can suggest a slightly less memory intensive algorithm.

StuXYZ 731 Practically a Master Poster

As Vernon says, you are very lucky that line 40 and 46 compile. HOWEVER: if they do, the (which in your case they obviously do). The actual error is this: cout << letterlist [3][0]; .

That is because possible is an integer an actually over flows!!

for (int n = dicerolls; n>0; n--)
    {
        table [dicerolls - n] = possible;
        possible = possible * 2;
        std::cout<<"possible == "<<possible<<std::endl;
    }

Look at the output from this modification.

Integers only have a certain number of bit, e.g. 16. 1 is used for the sign and the other 15 are for the value, hence why it crashes after diceroll > 15.

StuXYZ 731 Practically a Master Poster

The problem is basically simplified to this:

double **array = new double* [size];
array[i]=3.0;

What you have done is declare a array as a pointer to a pointer of arrays. That is effectively a double array like dynamic version of double A[10][20]; . However, you did not initialize the pointers to the arrays.

What you most likely want is this:

double *array=new double[size];
double *reverseArray=new double[size];
fillArray(array,size);
// etc....
// THEN ADD:
delete [] array;
delete [] reverseArray;

Note the rule that if you do X=new Something[Size]; you delete with delete [] X , and if you use X=new Something; then you delete with just delete X; . Almost every piece of memory that is allocated with a new should be deleted somehow. delete Note added after posting : Looks like Verion got there first, (obviously I type too slow ;) but note we are saying the same thing in slightly different ways.

StuXYZ 731 Practically a Master Poster

One of your problems is the error in CheckTitle:

bool CheckTitle(VideoGame*& first, string title)
{  
  VideoGame *current;
  current=first;
  while(current!=NULL)
    {
      if(current->Name==title)
	{
	  return true;                
	}
      else
	{
          return false;
	}
      // You can't get to here :
      current=current->link;
    }
  // If you get here what do you return ???
}

I have added a couple of comments, simply put you are exiting false if the first title is not good. The else { } can be deleted.

In addition, looking for a game title by exact name seems a bit harsh, how about if you look for the length of the input string, e.g. Halo matches. You could be slight more sophisticated and return a tri-state value from check title, unique/multi/none.

p.s. Did you know that if you put two lines like this

std::string A="a long line split in two"
         " with some extra";

// String B is the same as string A.
std::string B="a long line split in two with some extra";

That may help you readability in a couple of places.

StuXYZ 731 Practically a Master Poster

Ok the first and most important problem is simple that you have gone about this task in the wrong way. Start with the assumption that the compiler is your friend not your enemy, it is there to help, but if you write your whole program and then try to compile it you will have a very very hard time figuring out what you have done wrong.

So what you should have done it written the smallest compilable program that you can write and compiled, then fix the problems and then add a little more (basically in chunks of approximately one error per compile -- that changes with experience and later you can separate the compiler errors, you can write more, but initially that means no more than one method at a time).

So let us look at your code and figure out some of the types of error you are making.

First off, is that you seem to have no care for the capitalization of variables double avar; is different from double Avar; .
You consistently use random first letter capitalization.

Second you do this:

class A
{ 
   int hr;
   A(int);
};

// THIS IS WRONG:
A::A(int hr)    // THIS mask the class variable
{
   hour=hr;     // hour doesn't even exist anywhere
}

You should have written

A::A(int hour)
{
   hr=hour;
}

Word of caution, you write 06 and that is an octal number, i.e. 08 is illegal because there is no 8 …

anglwthnati2de commented: If only our Professors would walk us through this like you did, you wouldn't have people like me bothering you! (LOL) +0
StuXYZ 731 Practically a Master Poster

The problem is that you are passing a particular structure to your printstudent function BUT you are not actually using that data. Let me explain with an example:

int main()
{
   Student FirstStudent;    // Make a student
   Student SecondStudent;   // Make a DIFFERENT student

   // Now populate the student with infomation

   printstudent(FirstStudent);   // Print only about first student
   std::cout<<"Now write about second"<<std::endl;
   printstudent(SecondStudent);   
}

Note that in the above code, you call printstudent with the object that is required to be printed.
Not with a definition e.g. This is wrong: printstudent(Student& a); In the code the student writing should work with each student passed to the function.
in your function you do not do that. So change it to something like this:

void printstudent(const Student& SItem)
{
  cout << SItem.name << endl;
  cout << SItem.address << endl;
}

Note the use of SItem. That indicates that the object SItem's data will be used. You can equally use the same construct if you want to call a method of the Student struct for a particular object.

Hope that helps.

StuXYZ 731 Practically a Master Poster

It is another one of these typename instances.

First off the solution is this:

template<class t>
typename Openfile<t>::reference Openfile<t>::rout()
{
  return fin;
}

Ok that is the simple part, now I am going to make a complete mess of trying to explain WHY you need a typename. [sorry in advance].

The compile at this point when it sees somthing like this:

template<typename T> 
A<T>::X A<T>::Y() { }

Now what does that mean? It could be a method definition as you wish it to be OR it could be a constructor e.g. a complex form of

template<typename T>
class A 
{ 
   class X {};
};

A<T>::X object();

Now to avoid this ambiguity, you have to add typename. Note EVEN if you have the whole class there in the definition, this still holds. The basic rule is if you are using a template in a object type that is NOT a instance declaration then you need a typename.

StuXYZ 731 Practically a Master Poster

What doesn't work with this:

int
main()
{
  VStack<double> A;
  A.push(10.0);
  std::cout<<"A =="<<A.size()<<std::endl;
}

You remember that you have to use a template type in declaring VStack and that you might need to explicitly initialize the template if VStack and Stack are in separate files e.g. Add

template class Stack<double>;
template class VStack<double>;

HOWEVER: you may have made a complete mess, because you seem to have using namespace std; in your code, your names stack, etc are very close to the std names and you could easily be having real problems there.

StuXYZ 731 Practically a Master Poster

This seems a lot like you have coded before you have figured out what you are going to do. So let me point out a number of strange things that you may have overlooked.

First you initialize the board to 1,2,3, etc... e.g.

for(int y=0; y<n; y++)
    for(int x=0; x<n; x++)
      game_board[x][y] = value++;

Surely at the start of a game you are going to use say 0 to represent no counter, -1 for black and 1 for white. So you want to initialize the board to zero. By setting each point to a value, that is only used when you write out the board [and makes it almost impossible for the user to see the real game], you have made your life very very difficult.

Now you ask the poor used to enter the number based on the total position on the board. That is ok, but surely the coordinates would be better.

Now to calculate if you have a line of 5. Well first thing is you ONLY need to check
each of the positions from the point that you entered a new counter. Therefore,
you only have to check (at most) 8 directions. However, you do have to note that
a line can be formed up to 4 additional counters away from the point.

So the obvious way is to check from the point with a continuous addition: I will demonstrate the idea with a horrizontal forward direction line, …

jonsca commented: Nice one +5
StuXYZ 731 Practically a Master Poster

I think you have almost expressed your own solution, :). If the race is ALWAYS going to be less than 12 hours, or in some way constrained to be in some range which is less than 12 hours, then you simple calculate the time, and if the time goes negative, then add 24 hours.

However, if you can have competitors that taking 4 hours and others taking 20 hours, you are going to have to have some date flag. [you might like to do that say by looking at the start number, or the order, or simple putting the day.]

StuXYZ 731 Practically a Master Poster

You main problem is the algorithm that you calculate a continuous average with:
you do this:

for(int i = 0; i < Student::getCount(); i++)  
  for(int j = 0; j < 5; j++)
    avg[j] = (avg[j] + (userData + i)->getScores(j)) / i;

So what that does is effectively divide by i each time. e.g consider only 2 students with scores 3,3,3,3,3 etc. You do: ave[j]=((3/0)+3)/1). Obviously from that
expression you will get infinity.

The correct way to calculate an average, is to accumulate a total, and then divide by the number of students.

for(int j=0;j < 5; j++)
       avg[j]=0.0;
     for(int i = 0; i < Student::getCount(); i++)
       for(int j = 0; j < 5; j++)
	 avg[j] += (userData + i)->getScores(j);;
     for(int j=0;j < 5; j++)
       avg[j]/=Student::getCount();
StuXYZ 731 Practically a Master Poster

The obvious [once you have seen it] trick to make your output easy to read is this:

char first=' ';
for(int i=0;i<num_var;i++)  
{
  if (equ_1[i]<0)
    cout<<equ_1[i]<<variable_array[i];
  else if (equ_1[i]>0)
    std::cout<<first<<equ_1[i]<<variable_array[i];
  if (equ_1[i]) first='+';
}

You can note that you don't need a separate loop index for variable_array, and that using first, allows you to have a + between each number. It also deals with the classic problem that you normally don't want to write 0a+4b=10, but 4b=10. There are slightly more elegant ways to do this but it adds some advance concepts.

As to your original question, all you do is not that if you have eqn: a+b=40, it is the same as a+b-40=0, and you can treat the 40 as a value that doesnt have a variable, e.g. starting abc etc from equ_1[1] and reserving equ_1[0] for the constant

StuXYZ 731 Practically a Master Poster

This is a classic case of ignoring the compile warnings! It is 99.999% essential to turn on compiler warnings and fix the ALL. [Yes sometimes that means turning a particular warning off, but at least you are aware of exactly what caused it]

In your case you have a constructor like this:

Game()               
    {
      for (int i = 0; i < 100; i++)
	{
	  numb_array[i] = false;          
	}      
      guess_numb;         // This does nothing      
      win_numb;           // This also does nothing
      srand(time(NULL));
      int elct = 100;
      int found = false;           // this is a LOCAL copy of found that 
      int low = 0;                 // masks the copy in the class.
      int high = elct -1;            
      int middle = 0;
  }

you have created local copies that are only in existence until the end of the function. You compiler would have warned you about that.

So a much better way to do constructors, is to initialize objects/values in an initializer list. e.g.

Game() : guess_numb(0),win_numb(0),elct(100),found(0),
     low(0),high(elct-1),middle(0)
{
   for(int i=0;i<100;i++)
     number_array[i]=0;
}

But the MAIN reason that you posted here was that you either did not activate all the warnings/ turned them off/ or ignored them. By putting warning on, you will find lots of bugs and problems much quicker. The compile is your friend, it is much much more difficult to find errors at runtime than at compile time.

StuXYZ 731 Practically a Master Poster

It seems like it is actually some other part of your code now. You could run your code in the debugger. If you can write a simple test case for array2d, then post the whole code here. [At the moment, we have nothing that we can actually run.]


You can also put a simple try { // your code here } catch (std::exception& A) { std::cout<<"Exception == "<<A.what()<<std::endl; } , around the code in int main(). [Assuming that you are not using throw in your array2d code.]

StuXYZ 731 Practically a Master Poster

The copy constructor looks ok, with the exception that you must put a guard in for rows==0 and cols==0, but the assignment operator is a mess.

The problem is is that if you reassign memory you haven't deleted the old memory.
In particular you call the delete operator directly from the class, this is almost always a serious error.

This is much more likely to work.

template <typename T>
Array2d<T>&
Array2d<T>::operator=(const Array2d<T>& m) 
{ 
  if (this != &m) 
    {
       if (rows != m.rows || cols != m.cols) 
	 {
            for(int i=0;i<rows;i++)
              delete [] array[i];
            delete [] array;
            
            // Essentual in case m.cols or m.rows ==0
            array=(m.rows) ? new T*[m.rows] : 0;
            for(int i=0;i<m.rows;i++)
               array[i]=(m.cols) ? new T[m.cols] : 0;
            rows=m.rows;
            cols=m.cols;
          }

	for (int i = 0; i < rows; i++)  
          for (int j = 0; j < cols; j++) 
             array[i][j] = m.array[i][j];
    }
  return *this;
}

Also finally make sure that your default constructor is like this:

template<typename T>
Array2d<T>::Arra2d() : rows(0),cols(0),array(0) {}

or your delete operator will cause a problem.

Also I really think you should add a std::cout<<"rows == "<<rows<<" cols=="<<cols<<std::endl; to your destructor.

StuXYZ 731 Practically a Master Poster

As arkoenig said, as you have written it, it doesn't compile.

If you are using the construct operator new, you need to do it like this:

class X
{
  X() { std::cout<<"Constructor called "<<std::endl; }
  ~X() { std::cout<<"Destructor called "<<std::endl; }
};

int main() 
{
  X *x=new X;
  X *y= static_cast<X*>(operator new (sizeof(X)));
  delete x;
  delete y;
}

BUT be very careful: the constructor is only called ONCE. ie only for x, not for y. Be very very careful calling it, because if you don't have a POD [plain old data] type class, you are opening yourself to a nightmare of runtime problems.

StuXYZ 731 Practically a Master Poster

The mistake is almost certainly with the copy and assignment operators. You will get both, auto written by the compiler if you don't provide them. In this class both will be 100% wrong, as they do a shallow copy.

So either, prove that they don't get called: E.g adding

private:

Array2d(const Array2d<T>&);
Array2d<T>& operator=(const Array2d<T>&);

In the private section of your class declaration.

Alternatively, write a copy/assignment operator.

Additional:

Check what happens if you do actually call it with integer values ==0;

StuXYZ 731 Practically a Master Poster

I would like to see all of this class, well in particular the copy and assignment operator, as well as the default constructor.

The correct constructor should have:

arrayInstance = new T*[rows];
for (int i = 0; i < rows; i++)
  arrayInstance[i] = new T[cols];

the destructor should have

for (int i = 0; i < rows; i++)
  delete [] arrayInstance[i]; 
delete [] arrayInstance;

Just a quick note, you obviously don't use T** array; since you are using arrayInstance in the other parts of the code.

Finally, you can do this:

Other than that, your allocate and deallocator look ok, as long as (a) you correctly set rows, cols. (b) the memory is correctly copied, (c) any default constructor sets row and col to zero, and arrayInstance to zero.

StuXYZ 731 Practically a Master Poster

Almost any algorithm that calculates Pi, [or anything else] is almost completely unlikely to need true-random numbers. What you normally need is certain properties of the pseudo-random number generator. For example, if you decide to calculate pi by
simulating a 2x2 square (area 4) and a circle in that square [touching the sides], area pi. The determining the number of points in the circle, from points selected within the square.

A "pseudo-random" generator that divides each side in two loops, giving points
(0,0) (-1/2,-1/2),(-1/2,1/2),(1/2.-1/2),(1/2,1/2),(-2/3,-2/3) ,(-2/3,-1/3) ....
would be perfectly adequate, [and work very well].

The most important thing to consider the algorithm and the requirements of the correlation between the generators pattern and the algorithms result. However, this is often a very difficult thing to work out, so defaulting to a "highly" random pseudo random number generator, as previously suggested, is a good default position.
[Very popular at the moment is MersenneTwister type algorithms, which is a basis for some of the random number generators in TR1.]

StuXYZ 731 Practically a Master Poster

First off I would like to second SasseMan's comment, for very high precision you need the GMP.

Second your algorithms is going to kill you, you have 27111, copies of a function call on the stack, that is a block of memory for each function call. If you wish to use this algorithm please reformulate it as a loop!

BUT, we don't know g to that many decimal places, and g changes dependent on height.
Every 1mm of altitude change alters g by about 1e-10 m/s. So unless you know your altitude to +/- nanometers, long double is sufficiently accurate.

Also long double is defined by added an L after the number e.g. long double X(1.2L); . You should use that on you #define of g otherwise it will be converted to a double, which will lose you some accuracy.

Finally, you have a strange value of g, [assuming the problem is Earth bound], typical value [sea-level], is about 9.807m/s.

StuXYZ 731 Practically a Master Poster

Unfortunately, the a pile of little but important numerical/coding errors.

In arrMean, you don't actually get the wrong answer (I think), but you don't need to calculate avr until after the loop which adds up sum.

arrMedium: if (middle % 2) implies that the remainder of middle / 2 is not zero. i.e. middle is odd. In that case the medium is just the value at middle,
however, you have ALSO calculated middle without regard to the fact that the test should be for the arrSize, e.g. if you have say 11 items, then the middle is 5. The
test should be on arrSize.

Additionally, this is strange medavrB = arr[middle + (1/2)]; It actually works, but I guess you don't know that 1/2 is evaluated to zero, since it is integer arithmatic.

arrMode is simply wrong, you are going to have to take a piece of paper and work out that the mode is the most frequent number. Run your algorithm on paper and you will hopefully see what you can do.

Finally you need to fix your compile errors in main. The most important one is this arr[arrSize]=(1,2,3,4,5,6,7,8,9,10); is actually not what you think.
The comma operator evaluates from left to right and the round brackets are only there in a mathematical sense. It is equivalent of this: arr[arrSize]=10; As hopefully you can see that is wrong.

So use an array initializer or a loop.

StuXYZ 731 Practically a Master Poster

Can I just add, the code the OP has given will go horribly wrong at some point,
since int random_card = (rand() % cards.size()) - 1; can result in -1.

That will be a very strange output, and some memory violation later, most likely resulting in a core dump.

The -1 is not necessary.

StuXYZ 731 Practically a Master Poster

First off there is not enough code provided to actually compile this. However, if the only error was that you are getting array overrun I am absolutely surprised.

The likely error is to do with deletion, and the use of bare-pointers in std::vectors.

E.g. Consider struct Population. Now that takes a DNA pointer and delete it. You have no copy constructor so you will have simple copy by value system, therefore you will have multiple delete of each copy of DNA* pChrome. That is particularly important for vectors since there are often extra copies and deletes in the construction and push_back as the memory is reallocated.


I am terrified of what GeneticAlgorithm does in its deletion operator, m_v_parentPop2 is not deleted but m_vPopulation2 is?

Next you are changing the size of m_vPopulatioin2 by using push_back but not adjusting the other sizes, e.g m_iDensity. BUT then you are using the size of m_vPopulation as a loop control variable -- a certain error at some point, since m_iDensity is too small.

In all, you have a big base of code that simply is buggy and interconnected to sort out. The best thing to do is to start again, and put it together bit by bit, but test each section, e.g. use vectors for all arrays, and check that the size of ALL arrays match. Check that nothing is every double deleted, nothing is not deleted. Write a register class for memory locations, or use a …

Fbody commented: Nice catch on the pointer. +4
Ancient Dragon commented: nice help :) +34
StuXYZ 731 Practically a Master Poster

Why don't you capture the salient information before the decision tree. e.g.

struct TRes
{
  double pTime;   // time in seconds 
  double yVal;    // y positions
  A(const double Y) : pTime(time()),yVal(Y) {}
};

// Note this becomes the fixed point
TRes A(position);

if (A.pTime<0.08) 
  y1=A.yVal;
else if (A.pTime<0.15)
// etc...

You might need to have a set of if, since you seem to be assigning y1 AND y2 if the time is say 0.12. But that is just your algorithm requirements.

What you don't want is to access the time and position variable at different points
after each test etc. That will lead to having say t2 >=0.15 or something you don't want.

StuXYZ 731 Practically a Master Poster

Well first off, ALWAYS write a constructor, a copy constructor, an assignment operator and a destructor unless you are 100% absolutely certain you don't have to. If you have to think about it for even a second, write them all.

Therefore, what happens, well size is not initialized. [nor is list]. But you then allocate memory completely randomly. Also when do you get rid of it?

You MUST write a default constructor and a destructor and most of your problems will go away.

In addition: you are using namespace std; and then using a variable called "list". This is 99.99% certain to trip you up soon. Since you have a list object in std. Please just don't use that "using" construct, sure use a smaller form, e.g. using std::cout; etc , or just use std::cout.

You have errors in remove(), you copy list[y+1] when y can get to the end of the array. Obvious, memory corruption, particularly if your template type has its own copy constructor.

Finally, make print put a std::endl; onto std::cout when you have finished writing.

StuXYZ 731 Practically a Master Poster

I have looked at the code a little more. I also ran a simple test case, and there are a few problems.

The first is findNeighbours. That method goes beyond the boundary of the rows/columns.It produces an seg-fault when it does it [and if it didn't would give you the wrong answer. You also increment row and col in the function, and therefore get the wrong answer anyway. Also neighbour is not zeroed ....

You use true as a char test. BUT space is not tested, so you have to enter null characters into the input file... So assign the character to a space if it is empty [ or maybe a . ]. Then test for that and put an X in other spaces for example.

StuXYZ 731 Practically a Master Poster

Well first off, you can choose to use the abs function, or not as you wish. If you want to use the abs function, then do this

int a(93);
int b(102);
std::cout<<"difference == "<<abs(b-a)<<std::endl;

Note, that you will need to add #include <cmath> to your header.

You could also skip the abs function [by effectively coding your own] e.g.

int a(93),b(102);
diff=(b-a);
if (diff<0) diff*=-1;
std::cout<<"Diff == "<<diff<<std::endl;

You can also shorten the above with the ternary operator (? :), but that is not as readable to a beginner as the above (I think).

StuXYZ 731 Practically a Master Poster

Ok, first off, I REALLY hope this doesn't compile on VC++ because if it does then VC++ is adding a bracket
In addition to not compiling to the standard.

So what did you do wrong:

You missed a } at the end of insertAfter in SinglyList.h.

[The clue is that all the errors are in the same object, regardless of the fact that they should be in different ones]


Second: You are required to change to this

typename SinglyList<Object>::Position v = S.first();

The standard say that this causes the name to be treated as a type, and it also says that without the typename, it is treated as a non-type EVEN if this leads to a syntax error.

Thus add the typename directive. [You have about 4 places to do that, I think]

StuXYZ 731 Practically a Master Poster

That was the reason I wanted you to put an output after EVERY statement.

You have not yet understood the ordering issue in a program. For example, consider the variable c2. First it is created on line 20, It is assigned the value pow(b,2).
However, at that point b could be ANYTHING. That is because it had not been assigned a value YET. Thus c2 can be ANYTHING. Then later c2 is set to a2+b2. The assignment of a and b in line 26, is irrelevant. a2 and b2 have already been set, and are not re-set.

Please go through and write a cout<<"Variable == "<<var<<endl; at each line.

Programs are executed in an order, statement after statement, if something changes later, that does not effect the statement that has been executed. It is not a maths proof.

You need to rearrange your code, so that a,b,c are read from the input, before a2,b2 and c2 are evaluated, and before answerc is evaluated and printed to the screen

StuXYZ 731 Practically a Master Poster

Very sorry to tell you but the operator ^, which you use e.g. a2=a^2 is actually the XOR operator. ie. it compares the bits in the value and sets them according to the rule:

A  B  Out
0  0   0
1  0   1
0  1   1
1  1   0

Thus 1 ^ 2 == 3, 2^2 is 2, and 2^3 is 1. [You can check that by outputting a2,b2 etc.

So what you really needed was the pow function e.g. a2=pow(a,2); or to just multiply the two numbers together, e.g. a2=a*a; .

I note that you also use the comma and && operator in your output. I very much doubt that you want that. e.g. your cin line should be cin>>a>>b; .

What I recommend is that you put a lot of cout<<"Variable == "<<var<<endl; after EACH statement, to see what changes etc.

StuXYZ 731 Practically a Master Poster

You have made the classic error, that always seems to trip up beginners: That is if you use integers, then you get integer arithmetic :

Consider this code:

std::cout<<"1/4 == "<<1/4<<std::endl;

That will print the unexpected value: 0. That is because 1 and 4 are both integers and fractions cannot be represented in an integer value, therefore they are truncated.

In you code you sum up the result into a float. This does not help:

float F=1/4; 
std::cout<<"Look f is still 0 == "<<F<<std::endl;

That is because the division is done before the conversion type is examined.

However, this is different:

float F=1.0/4;

Since 1.0 is a double and the integer is converted into the double.

So in short use 1.0 instead of 1.

StuXYZ 731 Practically a Master Poster

Fundarmentaly you have two problems , (a) that you are using the wrong algorithm, (b) that you have put the multiply operator as a friend method, and not a standard method of the class.

class Polynominal
{
   // stuff
   Polynominal operator*(const Polynomial&,const Polynominal&) const;
};

is a better way forward.

Then you want to consider some simple polynomials and do the multiplication e.g (x^2+3x+2)*(x^3+3x)=x^5+3x^4+5x^3+9x^2+6x You get up to degree of first * degree of second polynomial terms. Therefore you are going to need a double loop e.g. one over the first array, and one over the second array of Monomials, and then you are going to have to add up individual terms [Note: If the sum of the term goes to zero it needs to be removed!!-- e.g. (x-1)*(x+1) is x^2-1.

StuXYZ 731 Practically a Master Poster

First off, yes you need to add a semi-colon. BUT in addition you need to note that GCD is a member function of Mixed and you have extracted the operator to an external friend function [more on that later].

Since GCD does not actually touch the class, or use any of the variables of the class, the best way to fix this is to make it a static function. e.g.

class Mixed  
{
  // stuff here
   private:
     static int GCD(int,int);
};

Then change line 31 to this: gcd=Mixed::GCD(r.numerator,r.denominator); Just a questions : sorry, I have just seen a couple of people making stuff like this: friend Mixed operator*(const Mixed& f1, const Mixed& f2); .

Just why do you need the friend directive. Without it, this would work perfectly

class Mixed
{
   // other stuff
  public:

    Mixed operator*(const Mixed&,const Mixed&) const;
};

Mixed Mixed::operator*(const Mixed& A) const
{
   Mixed B;
   B.numerator=A.numerator*numerator;
   B.denominator=A.denominator*denominator;
   return B;
}

Mixed A;
Mixed B;
Mixed C = A*B;

The only time you might need the friend class is if you have a second type e.g. Mixed operator+(const double&,const Mixed&) const; and want to write Mixed A; Mixed C= 0.6+A; . Your operator<< is such a method. [Although the friend is removed by making operator<< external to the class and adding a write method to the class]

Sorry to add a long comment that was not asked for, but it seems a very strange way to set the class up and I …

StuXYZ 731 Practically a Master Poster

Yes you should post life.h, well how can WE run the program, if you don't! -- and you want help with a runtime error!!

You seem to have made a mistake in iterateGeneration(). You create an initial line text that has zero characters in it. Then you call findNeighbours, however, then you get into a conditional test that can easily leave temp without any additional characters. THEN newtext is short.

That then causes a runtime problem on the next loop, since text is assigned to new text, and you then access text, assuming it is the correct length.

To demonstrate that: Add this to the end of iterateGeneration

for(int i=0;i<numRows;i++)
  if (text[row].length()!=numCols)
     std::cerr<<"Error with line "<<i<<std::endl;
StuXYZ 731 Practically a Master Poster

Well a little more information about the actual error message and the compile sequence would help.

However, my guess is that you forgot to create an instance of your templated class.
If you combine the code into one file, it works. [Not something to do other than for testing !!!!] . So add line: template class Cursor<char>; at the end
of your cursor.cpp file. This will create an instance of Cursor<char>.

If this doesn't work please post a little of the error messages, and how you are compiling/linking the program.


l

StuXYZ 731 Practically a Master Poster

Your problem looks like this line: scanf("%s/n", &ans); , since ans is a char. But you are reading it as a string. It is not. You perhaps want to do this scanf("%c",ans); .

The reason you get a crash, is that your text entry on the command line, is always at least two characters long. The first is the answer (y,n or whatever) and the second is the null character (zero) that is added to the end of the string to indicate it has finished. But you only have memory allocated for one character. So something else gets over written.

StuXYZ 731 Practically a Master Poster

Ok in the first instance, using power is plain ugly and inefficient. The typical route for a polynomial in the positive sense, is to do something like this:

int total(0);
int xpow(1);
for(int i=0;i<=degree;i++)
  {
    total+=coef[i]*xpow;
    xpow*=x;
   }

You can also do it via an internal bracket expansion: [this is effectively (((coef[N]*x+coef[N-1])*x+coef[N-2])*x+.... ))) ]

int total(0);
for(int i=degree;i>0;i--)
  {
    total+=coef[i];
    total*=x;
  }
total+=coef[0];

I like the second one slightly more.... but not alot in it. [Note the fact you are counting down in the loop, AND you cannot save a step by initializing the total to coef[0] before the loop. -- I only emphasize the last point as ran a 2000 CPU hour simulation with that bug in it :(. ]


Your display function is wrong because you write coef[0] twice, run the loop for(int i=1;i<degree;i++) . Note that it is normal to write polynomials high power to constant.

Your multiply: If you want to limit the degree to less than 100, then as well as a warning exit as well. Also have your resized coefs??? in temp do you zero coefs in temp ???

Note: Mike_2000_17 beat me to the post [must learn to type faster], sorry for the repeat, the second version of the evaluation code might be of merit, however.

p.s. Thanks for posting a good first post question to the C++ forum, e.g. code blocks, and sufficient information about your problem and what you have tried.

peterman.k commented: Very helpful +1
StuXYZ 731 Practically a Master Poster

Firstly, obviously you can see that you don't need any template specialization for this example. But assuming that this is a example, let us see what you did wrong.
It is just that sloppy practices that are allowed in non-template code are not allowed when dealing with templates.


(a) for a template function you must have a declaration .
(b) You cannot add a double to a pointer so start and stop should be int/unsigned int etc.
(c) you have to put the default initializer in the declaration.

// Put a declaration of the generic version [ALWAYS].
template<typename T> T sum(T*,int,int,T =T());

template<>
int sum<int>(int* array, int start, int stop, int init)
{
  int* stp = (array + stop);
  int* str = (array + start);
  while(str != stp)
    {
      init += *str++;
    }
  return init;
}

// If the type can be deduced then there is not need to write 
// the type
template<>
double sum(double* array, int start, int stop, double init)
{
  double* stp = (array + stop);
  double* str = (array + start);
  while(str != stp)
    {
      init += *str++;
    }
  return init;
}


int main()
{
  int a[] = {1,2,3,4,5,6,7,8,9};
  
  cout << sum(a, 2, 9,0)<<std::endl;
  cout << sum<>(a, 2, 9,0)<<std::endl;
  cout << sum<int>(a, 2, 9,0)<<std::endl;
}

Note that three ways to call sum. They are all allowed because the type can be deduced. If the template type was only the return type, then you would have to explicitly put …

Agni commented: Helpful +3
StuXYZ 731 Practically a Master Poster

Your biggest problem here is that you (a) are trying to do evergthing at once, (b) cell broad....

You seem to have overlooked that if you have a cell board; in your class definition, then you don't put cell in front of broad to used it. That is because your have already said it is a double array of cells.

e.g.

void minesweeper::displayBoard() 
{
for (int i = 0; i < SIZE; i++)
  for (int j = 0; j < SIZE; j++)
    {
      if (board[i][j].covered == true)
        cout << " ";
      else
        // You need to print something if the cell is NOT covered : 
        cout<<board[i][j].minesAdj;
    }
}

You HAVE to start with something simpler than this because basically, I don't think you understand what a definition is and what a declaration is. Please do something with say a simple class with one integer, and one method, get that to work, and understand how to have 5 different versions.

class A
{  
   int x;
   A();
   void doSomething();
};

int main()
{
   A XX;
   X.doSomething();
   A array[5];
   for(int i=0;i<5;i++)
      array[i].doSomething();
}

Until you are happy with that code, can write a simple constructor and a meaningful doSomething (hopeful, one that writes something out, maybe changes x etc. you are going to struggle.

All coding is about taking small steps to the goal, in your case you have taken one huge step, it is MUCH better to take a step of small steps so that you …

StuXYZ 731 Practically a Master Poster

Well done, you are correct that in many of your statements.

But note that i<a,j<b is NOT equivalent to i<a || j<b . Because in the first case the i<a test is ignored. e.g for(int i=0,j=0;i<10,j<5;j++,i++) stops at j=5, but if you write
|| then it stops at j=10.

Also, note that combining i and j in one loop can be exactly what you want in some circumstances, e.g determining the trace of a matrix, you only need the diagonal elements. However, more commonly you need two nested loops.

[Note: very often if you need only one loop, you would drop the other variable, and just use one]
e.g.

// Diagonal sum:
double matrix[5][5];
// .. stuff.
for(int i=0;i<5;i++)
  sum+=matrix[i][i];
StuXYZ 731 Practically a Master Poster

You MUST make sure that each call to a new , is matched by one and only one call to a delete.

So looking at your code, you put the delete after 289 before 290.

I really don't like the goto. It is called AFTER the program has finished, but, before you have exited the mainloop. Its effect is to prevent normal exiting of the program.

Calling a function from itself, is ok IF and ONLY IF, you have a way to get out.
This is wrong for example:

void function()
{
   // Stuff...
   function();
}

Were as this would be ok:

void function(count)
{
   if (count<0) return; 
   // stuff
   function(count-1);
}

Note the early termination in the second version.
What you have created is an endless loop, which allocates a small amount of extra memory each time through. Therefore I am unhappy with it. It is perfectly good, if you call it a set number of times, then exit. But make sure you have an exit.

StuXYZ 731 Practically a Master Poster

Sorry sqw, I was amending my post which your were writing your comment....

If you write beyond that edge of an array, the memory is likely to be used for something else. If that is the case, then your are able to change absolutely any variable in your code. It comes in a number of guises, some by mis-controlling a loop, others by not controlling user input and a buffer becomes overly full.

StuXYZ 731 Practically a Master Poster

Your problem is the assignment beyond the end of the array.

You wrote this for (int i=0,j=0; i<row,j<5; i++,j++) And that means : do i<row, ignore the result and test j<5.

Therefore when you get to this m[i][j]=2; you will definitely run m[4][4]=2; regardless of row.
That means you assign a memory location outside of m. (which is your particular case turns out to be K).

You needed to write: for(int i=0,j=0;i<row && j<5;i++,j++)