Posts by StuXYZ

This method seems strange to me .

The first thing that is obviously wrong is these lines:

 while((*bIter)<(*aIter)){
            bIter++;
        }

What happens if bIter is at the last position in the array.

So in short I would have written something like this

  std::vector<int>::const_iterator aIter=aList.begin();
  std::vector<int>::const_iterator bIter=bList.begin();
  if (bIter==bList.end()) return 0;

  for(;aIter!=aList.end();aIter++)
    {
      while(*bIter < *aIter)
        {
          if (++bIter==bList.end()) break;
        }
     if (*bIter == *aIter)
        cList.push_back(*bIter);
    }

Finally there is always the standard algorithms and you can do this:

#include <algorithm> 

//.....

std::set_intersection(aList.begin(), aList.end(),
            bList.begin(), bList.end(),                       
            std::back_inserter(cList));

I am not 100% certain of its speed but is it quick and easy to write and normally the stl is quick enough relative to simple hand coded stuff.

Notes:

I am very disinclinded to use auto when the algorithm only works for int and not doubles or other number types.

Second many may not like the break in the middle of the loop -- you can put it outside if you like.

Tiny typo --

Line 38 : s.setval(5.1,0);
Should have beens1.setval(5.1,0);

There are ways round this kind of error (scoping the two test etc) but in reality better naming is more important. I hate any name that can be confused with another name by one character missing/changed etc. However I guess you wrote this as a test code and didn't worry about it -- we all have done that!

Your code is nearly correct but I am not 100% sure that it is giving your what your want:
So here is your code with some comments -- just to make sure:

  // This line sets the random number seed. 
  // you only need this call once only in your code 
  srand(time(NULL)); 

 // This is strange :  -- your code produces a random
 // number between 1.0 and 2.0. This is because rand() returns an 
 // integer between 0 and RAND_MAX so the division gives you 
 // a number between 0.0 and 1.0  and then you add 1. 
 float   random1 = (float(rand())/(RAND_MAX)) + 1;
 float   random2 = (float(rand())/(RAND_MAX)) + 1;

// Now this is fine
x = a* random1 + b* random2; 

// this line is strange becauses random1 and random2 have not changed.
// I expected to see ANOTHER copy of this:
random1= float(rand())/MAX_RAND +1.0;
 y = c* random1 + d* random2;

If you test your code with* a and *b then same value as c and d, I expect you want a different value for x and y.

(note : don't forget your semi-colons!)

The chances are that your error is actually the accuracy of the return value from the dot product.

With floating point calculations you often get a little bit of an error, so you might be returing -1.0000000001 from your vector dot product. The quickest way to check that is to simply do

 std::cout<<"Should be zero: "<<Point1.dot(Point2)+1.0<<std::endl;

If you see a very very small negative number then that is why acos will return NaN. There are ways round this either simple tests (e.g. checing the number is >1.0 or <-1.0) or by using mathermatical identies that force the number into range regardless of the floating point roundoff error.

[Note: in this case I think that if A and B are unit vectors +/- roundoff error then (A.dot(B)/(A.dot(A)*B.dot(B)) is bound within +/-1.0 -- but I am not 100% and you should check this before you rely on it -- I just wanted to show the kind of things I was talking about]

The problem here is (a) the typo error with write *num1= num2; rather than *num1=*num2; (which doesn't actually affect the problem). And (b) what you are defining by const.

Let us start by looking at the definition of limit:

  const int limit=100;

That says limit is 100, and the compile [except in very special circumstances] will try to keep limit equal to 100 at all times, so you, the programmer have less difficulty in following your own (or someone elses program).

Then you write this:

 void passingAddressOfConstants(const int* num1 , int* num2)
   {
      // Stuff [doesn't matter what]
   }

This is ok BUT you have told the compiler that the address pointed
to by the variable num1 will be constant. Thus calling it with
passingAddressOfConstants(&limit,&limit) is 100% not ok, because you are allowed to change the second variable, which implies that limit will be changed.

However, the function is also illegal (simplified to):

 void passingAddressOfConstants(const int* num1 , int* num2)
   {
      *num1=20;   // compile error
   }

you promissed that the value pointed to by num1 will not change, and then you try to change it!

However, even if it is not possible to guess what is going to happen at run time this is legal:

 void passingAddressOfConstants(const int* num1 , int* num2)
   {
      int sum(0);
      std::cout<<"ADDRESS of num1 : "<<num1<<std::endl;
      for(int i=0;i<10;i++)
         sum+= *num1++
      std::cout<<"ADDRESS of num1 "<<num1<<std::endl;
      std::cout<<"Sum  : "<<sum<<std::endl;
   }

Don't expect a pretty …

I think you have made another mistake: Consider the line of code

 f.write((char *)&array[i][j],sizeof(array));

Now for your test example you have an array of [3][3], which if we assume you have 4 byte integers: sizeof(array) evaluates to 36.

Ok so you then take each point on the array in turn and write out 36
characters (9 integers). I am certain that is not what you wanted.

Try without the loop:

f.write(reinterpret_cast<char *>(array),sizeof(array));

Same is true for your read statement.

Sorry but you have steped into one of those nasty legacy areas from C.

There exists a function [in stdlilb.h which is included via cstdlib] long int random(). Thus the first of your errors.

To avoid this either (a) rename your functions e.g. random100 or something (b) give it an argument (e.g. random(const int N) (c) put it in a namespace.

Then we have the scary stuff:

You write

 for (int i = 0; i < ArraySize + 1;i++)
    {
        array[i] = randomInt();
    }

Hang on a second!! you only allocated ArraySize items for the array.... so now you are filling ArraySize+1 items. That is not good, and is corrupting memory!

Finally!!

Please stop putting using namespace std; at the top of your code!! Global things like random() left over from C, have been (slightly) mitigated by putting many things into the namespace std. So you have just pulled in about 2500 different names. Each one is waiting to cause a difficult to find bug. So if you want to use an item from namespace std. Either

(a) write stuff like this std::cout<<"This is some output"<<std::endl;
(b) Just get the bit you need e.g. put this code at the top
using std::cout; Obviously you need to add everything you are using e.g. endl etc...
(c) Put your using namespace std; in just small scopes (e.g. the function/method.

I prefer (a) over (b) over (c) ...but many will have a different order.

Your problem is one or two things:

(a) the line STOP in your input only has 11 characters in it.
(b) You don't check the input size in your code before cutting.

If you put this in your code :

operand = (opCode.size()>12) ? opCode.substr(13,6) : "";

the code runs to completion.

Obviously, I would also do the same for the instructions = ... line above.

You error seems to be in the copy constructor/assignment operator of way.

If you add :

Way::Way(const Way& orig) : 
  id(orig.id),name(orig.name)
{}

Way& Way::operator=(const Way& A)
  {
     if (this!=&A)
       {
          id=A.id;
          name=A.id;
        }
     return A;
   }

All works as I would expect. [Note you also have to add a declaration to the class definition of Way for the assignment operator.

So let us look at WHY that is:

The problem comes from the methog you use to write a wayNode to the AreaDataRepository. First off you write

void SetOnGoingWay(Way onGoingWay) { this->onGoingWay = onGoingWay; }

This isn't good! Way is a large class why are you not using references. This is what I would have expected to see:

 void SetOnGoingWay(const Way& onGoingWay)  
   {
     this->onGoingWay = onGoingWay;  
   }

So in the first, we have a local constructor (copy constructor) called to make onGoingWay and then an assignment operator called in the actual function to do this->onGoingWay=onGoingWay. The second is an assignment operator call. [Note: Most optimizers remove one of those calls]. But that means that your version of the copy constructor does not set id or name, but instead relies on the default std::string consturctor which gives an empty string.

Other problems:

While we are looking -- if AreaDataRepository is to be used as a singleton - then make the constructor, copy constructor and assignment operator private because as it stands you can circumvent your singleton requirement.

I made a slight error : the line: if (data[i]>0 || data[i]<101)
should have read if (data[i]>0 && data[i]<101).

Obviously I just typed the code into the post, I didn't actually compile/run it -- I will learn... :-)

So I suspect that you have a data value that is above 100 or below zero....

If that doesn't work can you give the full program that you are testing and we can have a longer look..

The first thing I see is that the frequency code is extremely inefficient. It has a double loop, i.e. you loop over 100 times over all the quanties. If you had 300 numbers to read then you would have 30000 steps!!

There is also a horrible memory error. Your array is 100 units long. but you index freq[100]. Since array start from 0. You only have memory up to freq[99]. [Note that the array index is a short way of saying how many steps away from the start item

I suspect that you have made the same error with the array data. That may be effecting your result, because data[100] may contain corrupted data.

What about this:

int freq[100]; 
for(int i=0;i<100;i++)
   freq[i]=0;

for(int i=0;i<quant;i++)
  {
     if (data[i]>0 || data[i]<101)
       freq[data[i]-1]++;
     else
        {
           std::cout<<"Data out of range :"
               <<data[i]<<std::endl;
        }
  }

That gets your frequency a bit better...

Now to the actual question:

What you have their is a mess, if you have let use say this:
3,3,3,4,4,4,7,7,7. Then you would want the average of 3+4+7.
So you would need to (a) store the running total, (b) store the number of contributions to the running total, (c) store the number of items in the biggest mode value, so that if you then had this : 3,3,3,4,4,4,7,7,7,8,8,8,8, you would discard the running total of 3+4+7, in favour of just 8.

// Declare and initialize our three variables:
int modeCnt(0);   // Count of blocks contributing to sum.
int …

It certainly doesn't defeat the point of using templates, that is that you write one function and have it work for multiple types.

The simple template form can be resolved. It is just here that the compiler needs some help because it cannot know which one you ment. [I have figured out that my previous example was a little obtuse: sorry]
Consider: FILE 1 with this code:

template<typename T>
struct A
{ 
    typedef int XInt;
};

Consider that you write something like this:

// this can be in a .h include file
template<typename T> void func(typename A<T>::XInt&);

// this can be in a .cpp file
template<typename T>
void func(typename A<T>::XInt& x)
  {
     std::cout<<"x == "<<x<<std::endl;
  }

BUT in another file, somewhere else you write this:

// Specialization for when <T> is double
template<>
void func(typename A<double>::XInt& x)
{
    std::cout<<"double == "<<x*x<<std::endl;
}

Now the poor compile sees you code

int main()
{
   int i;
   func(i);    
}

What can it possibly tell about the intent that you had. It can't know. Both versions correctly match, and it doesn't know that maybe in another file somewhere that will be discovered at link time, or written by another programmer linking to your library
is a specialization of func<strangeClassType>(int&). So it complains and throws an error.

You can easily replace struct A by std::vector, and you can see that there are hundreds of different std::vectorthat have identical std::vector<int>::iteratorbecause you can specialize vector by the allocator. That is why …

Sorry it is a error -- not sure it is a beginners error, but it is an error type I think you should know about.

Solution is to write

display<int>(myBeginIterator,myEndIterator);

Reason:

What you have done is not give the compiler enough information to determine the type of the iterator. That comes about because iterator itself is a derived templated type. There is insufficient information to determine the exact type.

I have a simplified understanding of that (anyone else want to chip in with an explaination please do).

What happens is that you want a class value type. Let us write a simple one :

template<typename T>
struct A
{
    typdef int XInt;
    typedef T* XPtr;
};

Now it is perfectly possible to use XInt like this: A<double>::XInt i(20);

And obviously you can use it like this:

 template<typename T>
 void func(typename A<T>::XInt& x) { }

Everything is ok (at the moment) but the problem comes in that <T> does not actually define what you are getting as the type of XInt . You can get multiple versions of the same type, when all the compiler actually can resolve is the base type (int in this case). Thus to the compiler it is ambiguious which is forbidden. So you have
to specify e.g. int i; func<double>(i); func<int>(i);

Hope that helps, if anyone would like to make that clearer or point out the errors in my simple understanding please do.

Your errors are dominated by overflow. These kind of functions are normally computed in a logorithmic maniford, not the real number manifold.

Consider: pow(x,254) almost always in overflow for most of your x range.
pow(M_E,-x) at underflow for most of your x range

Those two can be combined: exp( log(x)*256-x), that reduces the over/under flow, and obviously doing the whole problem in the log space reduces it still further.

Your range from x to 100000 is going to do that unless you convert the integration to a log basis, if you calculated log_chi_cdf, you should be ok -- then just convert at the end

There are many things that are wrong. I am not figuring all of them out for you but will give some help.

(a) Consider the test case: 15X2 Expected output 3 (because 30 goes to 3)/ Actual output 1.

So what went wrong. It is in your multiplication. You don't keep any account of multiple digit number e.g. you do 1x5x2 and get 10 which is wrong.

(b) The specification says that you will need to deal with numbers up to 1e19. That doesn't fit in an integer [on most computers].

Overall your algorithm needs a re-think. Sorry.

You have to figure out a way that you can truncate the multiplication number or ignore the parts that don't matter. That can be done (for example -- there are many ways), by noting that 10 has prime factors 2 and 5.

-- Parse a result with zero in it as special
-- Remove the factors of 10 (e.g. a 2 and a 5 prime factor). You are left with a key result:
if you have a remaining factor 5, your result is 5
if not multiply all the last digits together and keeping only the last digit at each stage

The above is definitely not optimal!! It is an example that can be shown to work with very very large input.

The most scary thing about the above code is the pow(double(-1),j+1) ...
please think, an alternating pattern is required.

I initial thought that it was homework help, but it is actually nicely formatted obstifacted C++. So am not sure that the code deserves the -1 reputation. :)

What I find strange is the switch/case statement when you have a choice base on a simple number.

if (choice<1 || choice>maxCandidates)
  votes[choice-1]++;
else
  { 
     std::cout<<"Incorect number... Exiting"<<std::endl;
     /etc...
  }

Then the next thing that look strange is calling votes1() from votes1(). Surely
write a function getVotes() and then do the decision and processing outside that
function in another loop/control structure.

Finally, if you define optimization you have to specify what you mean, IO, CPU, size, or what I think you really mean, making the code clear.

First off, the reason for your error might be better understood if we consider this piece of code:

bool function(int a,int b)
{
   bool retValue;              // Value not set!!!
   if (a==b) retValue=true;
   return retValue;
}

That is in-essence what you have done. You actually hide it a little by not putting a
return statement at the end of your code, but it is the same code. If you need to see that, take your original and ADD a line after the last statement before the } in isanagram std::cout<<"HERE"<<std::endl; and you will see that you are then going to exit the function without an explicit return statement.

As to debugger, yes you will need to learn to use one but not now. Understanding, how to put print statement (std::cout) and fundamentally looking at your code will help more.

The error is in the function isanagrams. If you turn on the warnings on the compiler.
[No I don't know how to do it on Visual studio.] You will see that you are told that
fro that funciton: warning: control reaches end of non-void function.

The problem is that the value is the undefined, so you many get a true even if the strcmp(a,b) is not equal to 0.

Try changing the conditional to this: return (strcmp(a,b)==0); Additionally, you read the dictionary words into a VERY short character array (20 characters). I made the mistake of testing the program with a typical English dictionary file, and core dumped the program, because the longest words in the english dictionary are over 20 letters long. It is often important to be very careful about reading strings directly in to character arrays without checks on the maximum number of character that are allowed to be read.

Several problems exist.

First if you reference an array out of bounds you are going to get a mess.

Consider your code:

for (int i=0;i<V;i++)
    {
      for (int j=0;j<H;j++)
	{
	  ne=0;
	  if ( A[i][j-1]=='*' ) { ne+= 1; }
// .....

What happens as j is zero?? Similarly what happens when i is V-1 or j is H-1. You are then accessing unknown memory locations.

The solution to your problem is the define char A to be char A[V+2][H+2]; . Then set the boundary to be dead and then work from 1 to V and 1 to H.

Next Error:

You update the map on a cell by cell basis. The game is normally set out in a form by which the state of the game at a point in time is used to calculated the change, and then the whole change is implemented at once across the whole map. This can be easily carried out by having two arrays and switching between them each move.

There is nothing intrinsically wrong with your code. i.e. in a stand alone test env. it works.

The error you are getting comes from your library matrixOptLibrary.h. There are any number of reasons that this code is going to break it. (a) the using namespace std; ,
and more likely (b) that your typedef is in a include file and it is not in a namespace, and it is above an #include "matrixOptLibrary.h".

If you can post a small complete test code that produces the error, we will have another look. Otherwise pay particular attention to your .h files, particularly if you also have #include lines in them.

Sorry to say, it is the usual problems.

Basically, if you get a problem like this put some std::cout statements in the inner loops that are doing stuff. .e.g in reverseArray.

You would find: for(count=5;count = 0; count--) is wrong since I think you intended to write for(count=5;count != 0; count--) .

Note: That error would have been a warning from your compiler if you turn on the warnings.

You have another error, that is a bit more subtle, that is you have an array of size items, BUT you set count = 5. That means you have a bug waiting to happen. Next, you then use score[count] . Note that score is an array from score[0] to score[4]. Try something like score[count-1] Finally, please remember to delete the memory you allocate in reverseArray.

It is not strange that it runs, however, the problem is that exitMinutes is uninitialized.

That means that when the value in exitMinutes is read, it could be ANYTHING. To see how that came about, consider your program.

In the main program you write int exitMinutes; which creates a variable called exitMinutes BUT does not actually set its value. So it is whatever that memory location previously had in it.

Then you have this:

void inputAndValidate(int entranceHourP, int entranceMinutesP, 
                       int exitHourP, int exitMinutesP)
{
	do
	{
	cout << "Enter the hour and minutes the parking garage was entered (each separated by a space): " << endl;
	cin >> entranceHourP >> entranceMinutesP;
	}
	while((entranceHourP > 0) || (entranceHourP < 24) && ((entranceMinutesP > 0) || (entranceMinutesP < 59)));

Now under NORMAL operation, you pass a value to the function and then set the values before you test it. Fine, BUT what happens when you fail to enter numbers into cin, but instead, enter letters (for example).

The line cin>>entranceHourP >> entranceMinutesP; exits WITHOUT setting entranceMinutesP and as std::cin sets its own error state. THEN you use it uninitialized in the while loop. Hence the compiler warning/error.

Finally, the values you pass to the function are completely unused, you could just as easily write void inputAndValidate() and define your variables below.

Well appart from the strange layout, it works. That is always good.

Do note that a*a is the same as pow(a,2).

It is also a lot more normal to do the calculations in the double form. E.g. pow(a,2.0); . It is often quicker as well, since the floating point unit is normally in double.

Finally, think about the fact that often the points are not on integer values, what if I want to test the point 0.5,0.5,1.0 ?

You would normally use tmp variables since it makes it clearer e.g. I would be inclined to write a function that returns a value, e.g. distance. That way you can say use if for determining if two spheres intersect.

double distance(// variables here )
{
const double xd=a-x;
const double yd=b-y;
const double zd=c-z;

return sqrt(xd*xd+yd*yd+zd*zd);
}

In addition to gerard143's solution, which is fine. You may want to make getitsvalue virtual.

This is problem is a specialization of the common construction:

class Base
{
   public:
    // Stuff... 
    virtual void write(std::ostream&) const;   
};

class Derived : public Base 
{
   public:
   virtual void write(std::ostream&) const;
};

std::ostream& operator<<(std::ostream& Out,const Base& A) 
{
   return Out<<A;
}

I only really bring that up since it is so common, that you will see that construct in many different books and open-source programs. [Note the operator<< is sometime templated with all the different classes in the program.]

There is a third part the equation here: To illustrate consider this:

class AClass
{
  public:
    AClass(const int,const int) { std::cout<<"Constructor"<<std::endl;}
    AClass(const AClass&) { std::cout<<"Copy Constructor called"<<std::endl; }
    AClass& operator=(const AClass&) 
       { std::cout<<"Assignment operator called"<<std::endl; 
         return *this; 
       } 
};

int main()
{
  
AClass Object(1,1);        // Object constructed with constructor 

Object = AClass(2,2);      // ASSIGNMENT operator called.

So it the case that you have given then the assignment operator is called.

I am assuming that you have written an assignment operator. If you haven't do so now and for EVER other class you have, and while you are at it the destructor. C++ writes you an assignment operator and desctructor and copy constructor if you don't provide one, BUT they are shallow copies and if your class has any kind of complex object you definately need to write your own. The Rule is:
If in 1% doubt about needing to write the constructors/destructors/assignment operator write all 4.

In addition, I see that TheClass is derived from Parent, but you don't call its constructor in your copy consstructor. That is almost certainly unclear if not wrong.

Finally, since somethings like m_NumberOfPixelsInRegion are not constant, you are most likely going to have to write this construct for the assignment operator

TheClass&
TheClass::operator=(const TheClass& A)
{
   if (this!=&A)
     {
        Parent::operator=(A);
        m_NumberOfPixelInRegion=A.m_NumberOfPixelsIRegon;
        // ... etc ...
     }
   return *this;
}

There is always the ostream methods e.g.

void print_list_four(const std::list<cProg3>& lst)
{
  copy(lst.begin(),lst.end(),std::ostream_iterator<cProg3>(std:cout,""));
}

However, you will want two things, (1) to change your void Print_f(fstream &Outfile); to something slightly more general. void Print_f(std::ostream&) const; and (2) to add this functions

std::ostream&
operator<<(std::ostream& OutStream,const cProg3& A) const
{
   A.Print_f(OutStream);
   return OutStream;
}

Effectively you are giving your class a operator<< and then using with a functional.

However, there are a lot of ways to do this, and the difficulty is choosing the most appropriate.

The code that you wrote correctly works in both cases because you didn't actually do anything with obj2 or obj1.

Note you wrote this: cout << "ob2.tab[0] = {" << obj1.getTab()[0] << "}" << endl; That should have been obj2.getTab() not obj1.getTab() Additionally you have incorrectly written the constructor: This should core dump in most circumstances e.g. you have written:

A::A(double t[], int size)     // size MASKS the variable size in the class 
{
   tab = new double[size];
   for (int i = 0; i < size; i++)
    tab[i] = t[i];
}

It should have been written like this:

A::A(double t[], int s) :
  size(s),
{
   if (size>0)
     {
       tab = new double[size];
       for (int i = 0; i < size; i++)
         tab[i] = t[i];
     }
   else
     tab=0;
}

THEN if you remove the copy constructor, you get a core dump BUT it occurs on the deletion operator of obj1. To understand that, you must note what happens if you don't actually provide a copy constructor. The compile substitutes a simple copy constructor that makes a shallow copy of everything. In this case, the variable tab, is just copied, i.e. both obj1.tab and obj2.tab point to the same memory location, however, obj2.tab is deleted and everything is ok, [you have had one allocation and one deletion.] The get tab[0] command actually works, since there is no check to see that the memory hasn't been deleted, and nothing else has happened for the program to fill/change that memory. Then …

Well you really are going to have to give us a little more code. There are no obvious things that you are doing wrong:

#include <iostream>
#include <iomanip>
#include <sstream>
#include <string>
#include <vector>

int
main()
{
  std::ostringstream cx;
  typedef std::pair<int,std::string> PT;
  std::vector<PT> XX;
  for(int i=0;i<10;i++)
    {
      cx<<"a";
      XX.push_back(PT(i,cx.str()));
    }
  // Erase some stuff:
  XX.erase(XX.begin()+3,XX.begin()+5);
  // Write it out:
  for(unsigned int i=0;i<XX.size();i++)
    std::cout<<"XX == "<<XX[i].first<<" "<<XX[i].second<<std::endl;
}

The above code deletes a section from the vector and shows that the strings haven't been deleted. If that works on your system, you are going to have to enrich that program until it has your error.

Have you checked that you are not miss using an iterator e.g.

std::vector<PT>::iterator Iter;
for(Iter=XX.begin();Iter!=XX.end();Iter++)
  {
     if (Iter->first==3)
       XX.erase(Iter);   // THIS INVALIDATES Iter.
  }

Beyond that you will have to post a bit more code. sorry couldn't be more help.

I expect that you wrote somthing like this: std::cout<<"result == "<<a<<std::endl; . The problem with that is the default precision, for output, is 6. The number itself is of a higher precision.
If you write this

#include <iostream>
#include  <iomanip>   
 
int main()
{
double a=1.2345678901234567890;

std::cout<<std::setprecision(20);
std::cout<<"results ="<<a<<std::endl;
// Not all numbers are now in precision 20.

The following code give you something like : 1.1234567890123456912
That is because [for my 64 bit machine -- sizeof(double) == 8 bytes] the precision of double is about 14/15 decimal places. [and I got lucky with the roundoff for the 16th ;) ]

You will get more decimal points and a bigger exponent range if you use long double. If you need a larger precision for something, then there is the gmp [gnu multiple-precision library] http://gmplib.org/, which is only limited by your computer memory and speed.

Overall I am normally very very disappointed with the way that formating is done in C++. I basically rely on the boost::format http://live.boost.org/doc/libs/1_39_0/libs/format/doc/format.htmlsystem which is much more printf like but with slightly more type checking, and slightly better error messages.

Sorry the solution given is just too inefficient. You simple don't need to copy the whole array each time.

Taking ravenous's code base:

const unsigned int range = 100;
const unsigned int numberToSelect = 10;


int* remainingNumbers = new int[range];
int* chosenNumbers = new int[numberToSelect];
 
// Initialise the list of possible choices
for(int i = 0; i < range; i++)   
  remainingNumbers[i] = i;
 
for(int i = 0; i < numberToSelect; i++)
  {
    const int selectedElement = rand() % (range - i);
    // Remember the value of the chosen element */
    chosenNumbers[i]=remainingNumbers[selectedElement]; 
  
    // Now: ONLY exchange the selected number with the last valid number
    // in the remainingNumbers set:
    remainingNumbers[selectedElement]=remainingNumbers[range-i-1];
}
delete [] remainingNumbers; 
/* Do things with the chosen numbers */

/* tidy up */
delete [] chosenNumbers;

That is more efficient. It still has the uniqueness, and is still equally likely to select all values. -- Just in case that this not
clear:
Consider the number 1-5

Array: 1 2 3 4 5
// select number 2 (from  rand % (5-0)
Array: 1 5 3 4 (5)         // 5 at the end if repeated since we copied it in
// select number 3 ( from rand % (5-1)   -- you can't get the last number in the array.
Array: 1 5 4 (4) (5)       // 4 and 5 repeated
// select 5           // FROM position [1] from rand % (5-2)
Array: 1 4 (4) (4) (5) 
/// etc....

Can I also say that it …

The first thing you need to do to fix this, is to realize that you are passing WHOLE strings and not individual characters:
Add this line like this:

while(iss >> str) 
    {
      std::cout<<"str::"<<str<<"::"<<std::endl;        // ADD THIS LINE
      //iss >> str;

That will show you that you have str that is a complete string, it is not individual characters. E.g. When you do the test str!="+" it will ALWAYS be true, since "4+3" is absolutely NOT "+". Thus your code simply think you have a single object, and the postfix and prefix versions of a single object/number are both the same, just the object.

When you put str to be an individual character (e.g. char). You got a whole pile of errors. THAT IS GOOD!!!! That is the compiler helping you fix your code that is simple wrong in many places.

So, in short, make str (a) a char , (b) a better name, that you are not going to mistype and find is a keyword. (c) fix all of the compiler errors and then try a little debugging, e.g. putting output statement and assert's etc.

Sorry that is a little harsh but that is the only way you are going to fix this code that I can see. When you have fixed the compiler errors, feel free to repost, I/others will happily help with the remaining runtime bugs.

Finally, about putting a qualifier on on the line T tmp = stack<T>::top(); . …

First off welcome Daniweb.

Next, now that I have your code, what are the problems with it. Unfortunately, quite a few -- I have listed a few to get you going in the right direction.

The first is that you are using this code [lines 32-34]:

cin >> line;            // get a string into line up to the first space deliminter ] 
getline(cin, line);     // This reads the rest of the line 
istringstream iss(line);

That means that for the program to accept say 4*5 you have to enter: someJunk 4*5 . All the characters in the first expression up to the space ("someJunk") are thrown away.

So fix it by removing the cin>>line; Unfortunately, that isn't your only problem:

while(iss >> str) 
  {
    // Stuff...
  }

Since str is a string [not a char] then it assumes that you are entering you text like this: 5 * 4 + ( 6 - 32 ) .

The obvious way round that is to use a char. There are a lot of changes to that, you need to make the "+" into single character e.g. '+' .

Next you are using a very strange construct for handling the brackets

else if(str == ")") 
  {   //If the ) is encountered, pop the stack until the ( is found
    while(str != "(") 
      {
        // THIS LINE doesn't change the str so it is an infinite loop:
        // Until stack thows an exception.
	cout << stack.pop();
      }

Obviously, it would be a lot easier to look at this if you had posted all the code.

However: Find_Trav_Subtree looks very strange [to me].

You see it calls the right branch if (current->Right) AND then always returns Find_Trav_Subtree from the right branch REGARDSLESS of it returning a NULL. I would have expected to see something like this:

BSTNodePtr Result(0);
// Note only return from right if a valid result is found:
if ( Current->Right && (Result = Find_Trav_SubTree(Current->Right,ID)) )
   return Result;
return ( Current->Left) ? Find_Trav_SubTree(Current->Left,ID) : 0;

There are many ways to do this.

Hope that helps, if not please post the full BSTClass so we can also compile and debug it.

There are several mistakes in the above code. The main one is that you wrote the code in one long block without debugging each section. It is almost always easier to write the code in little bits, keeping a running program that does a little more each time.

Let us look at some of your common errors: Consider this code

while(i<phraseLength&&!letterFound)
   {
      if(nextLetter==phrase[i]);
      letterFound=true;
      i++;
   }

The first thing I did with your code was compile it with all the warnings on:
and I get several warnings and it tells me that I have an empty body in the if construction at 160.

The second thing I did, was reformat it with a code indenter. . That shows the problem. The if statement has an extra semi-colon so letterFound is ALWAYS true.

I am sure you can see your mistake now.

EDIT: theLamb beat me too it, sorry for the repeat, . But please compile with warnings and use an automatic code indenter. It really really helps show you were the errors are, just because things "look wrong".

On reading this I thought that the obvious solution is to use a functional.
The functional can be defined as a member of your class, or not.

Additionally, I wasn't actually sure what algorithm you were implementing, in this example I consider that you have an vector of values, and wish to find the positions of the lowest values without disturbing the vector.

consider if you did this:

class comp
{
  const std::vector<double>& RefVec;   

  comp(const std::vector<double>& A) : Ref(A) {}

  bool operator()(const int A,const int B) const
  {
    return (RefVec[A]<RefVec[B]);
  }
};

int main()
{
  std::vector<int> Index;
  std::vector<int> Out;
  std::vector<double> Value;
  
  for(int i=0;i<10;i++)
    {
      Value.push_back( ((i % 3)-1.0) * i*i);
      Index.push_back(10-i);
    }

  Out.resize(3);
  partial_sort_copy(Index.begin(),Index.end(),Out.begin(),
		    Out.end(),comp(Value));
 
  for(int i=0;i<3;i++)
    {
      std::cout<<"Lowest Values == "<<Value[Out[i]]<<std::endl;
    }
}

The function idea can be extended to include very complex ideas, since the functional has state.

Next: You can use member pointers to functions, but you need a calling function:
You just need a slightly different syntax. However, it wont help you (easily) here since you are going to have to do a lot of bind2nd etc to get it to work [or use boost::bind].

Note the comment the compiler gives you is correct, the key phase is address of an unqualified which is because you missed out the className:: part of the qualification.

class X
{
  public:
  // Two functions with same signature [ void () const ]
  void call() const { std::cout<<"CALL"<<std::endl; }
  void call2() const …

Sorry I am not impressed with your teacher.

Most compilers of c++ and a lot of other compiled language split the program up into individual files. Each of those files are normally compiled into object files.
These normally have the .o extension. They are the machine code of each block of code. However, in many of these .cpp file, there is a call to a function that is declared (normally in a .h file), but not defined (the actual code for a function, in a .cpp file. The object file contains a symbolic link to that function but not a direct call. These is resolved by the linker that takes all the .o files and tries to resolve all the symbolic links to produce a single executable.

Normally the production of an object file is done first, and then the object files are linked together. The commands given would be

g++ -c Client.cpp
g++ -c Server.cpp
// All the other .cpp files etc....

The you would link it with the command g++ Server.o Client.o other.o and that would give you an executable called a.out. [if you used the -o option you can change that name]

However, for short programs, there is a quick way, just give g++ the list of files to compile and it will compile and link them .

Your command line was : g++ hw2.cpp

So unfortunately the linker says that it cannot link the files because it doesn't know …

In this particular case, it seems to be that you are getting the exception from the line if (data[i].at(it->key) == check_word_key) . That is because it->key DOES NOT correspond to the size of the vector data.

This occurs because data is not a vector of size TABLE_SIZE but is of the number inserted. E.g. you insert keys 34, 56 and 78. Now you have three vectors with one component each. THEN you access from key 34, and you do data[34].at(34). Opps error.
Since data[34] only has one component.

To see this, modify your lines:

std::cout << "iteration through FOR loop" << std::endl;
std::cout<<"Size of data["<<index<<"]=="<<data[index].size()<<std::endl;
std::cout<<"Accessing "<<it->key<<std::endl;

You have made the same mistake as the original [in a different place].

You bool operator does not know anything about the vector or the type, that is because it actually is the type contained in the vector:

it should be this:

bool operator==(const int& Index) const   // Get the value to be compared into index
{
  return (key==Index);
}

You have completely confused yourself by using the variable name key in just about every possible location in your code, thus it is a scope understanding master-class to figure out what is going on. Try this:

template <class RecordType>
bool table<RecordType>::is_present(int possibleTarget) const
{
  std::size_t i = hash(possibleTarget);  

  typename std::vector<RecordType>::const_iterator it;

  for (it = data[i].begin(); it != data[i].end(); ++i)
     {
       if (data[i].at(it->key) == possibleTarget)
          return true;
     }
  return false;
}

Finally, get hash() to return a reference to a data object and not an index value, you can use an empty vector for a "not found". [When you have it working!!!]

you have two problems with your code:

(a) missing typename on the in the template file.

std::vector<RecordType>::const_iterator it;  // THIS IS WRONG

// This is correct:
typename std::vector<RecordType>::const_iterator it;

There are many explanations around for this, it basically is because since depending on what std::vector<RecordType> is specialized as, you can't figure out what const_iterator will be. [It might resolve to a value for example].

Next you have a small issue in resolving the repeating name issue and
in precedence order:

if (data[i].at(*it.key)  == key)  // THIS IS wRONG
// This is better [precedence order correct]
if (data[i].at(it->key)  == key)

That will leave you with one function to add, and that is a bool word_record::operator==(const int&) const; since you are comparing a RecordType with an int on the line 15 of the code.
you might like to write the completely unintelligible if (data[i].at(it->key).key == key) but then I don't think that is a good idea.

Additionally , you need to provide implementations of table::table and table::insert().

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.

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.

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 …

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.

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.]

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();

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]

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.

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 …

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