Banfa 597 Posting Pro Featured Poster

Which line of your code reads the 19?
What do you do with that 19 after it has been read?
What will your code do if your professor decides to test it on a file containing 20 values?

Banfa 597 Posting Pro Featured Poster

The problem is that there are lots of corner cases which is why you have to keep on recalculating number of trucks and expected average because you have to keep on adjusting your goal for what you have already done.

take this series of loads

10, 35, 15, 20, 10

On the first pass
Total load = 10 + 35 + 15 + 20 + 10 = 90
Trucks = 90 / 40 = 3 (rounded up)
Average Load = 90 / 3 = 30
Assign 10 to Truck 1, 35 wont fit into the truck so start pass 2

Pass 2
Total load = 35 + 15 + 20 + 10 = 80
Trucks = 80 / 40 = 2
Trucks has dropped by one calculation on track for 3 trucks
Average Load = 80 / 2 = 40
Assign 35 to truck 2, 15 wont fit in onto pass 3

Pass 3
Total load = 15 + 20 + 10 = 45
Trucks = 45 / 40 = 2 (rounded up)
Trucks has NOT dropped by one calculation not on track for 3 trucks however it is not possible to fit anything else into trucks 1 or 2 so we must actually be look for a 4 truck solution even though our initial estimate was 3
Average Load = 45 / 2 = 23 (rounding up)
Assign 15 to …

Banfa 597 Posting Pro Featured Poster

Sorry I had not fully realise the implication of rule 3. However I think something like this should work.

From the complete order list build a list of orders or work on. This sub list is bounded by any of the following conditions, the start and end of the main list, a split order. From the second example 18, 18, 8 would be a sub-list because it is bound at both ends by sub-lists.

Calculate the number of trucks required: (18 + 18 + 8) / 40 = 1.1 = 2 (because you can't have 0.1 of a truck)
Calculate the average load for each truck: (18 + 18 + 8) / 2 = 22

From the list fill the first truck until you are as close as possible to the average.
Truck 1: 18 because abs(22 - 18) < abs(22 - (18+18))

If there is 1 truck left put everything else into that truck
Truck 2: 18, 8

Else recalculate the new number of trucks and average.

If the number of trucks has not gone down by one then move up the stack of trucks until you can shift 1 load back into a previous truck. And restart.

Otherwise start filling the next truck.


I think you will need to do this recursively because I think there will always be possible load groupings that you will be unable to determine the correct layout for by a single pass. A Slightly tricking set of loads is

19, 20, 22, 24

I'll explain more if I get time later.

Banfa 597 Posting Pro Featured Poster

Actually prism does not need to be a template. What it needs to do is store a shape* that points to a shape that is the cross section of the prism.

That would let you change the cross section if you wanted to.

Note that this pointer should probably point to a shape constructed specifically for use by the prism so that would need to be allocated on the heap.

Also when passing objects into functions always use either a constant reference const shape& or if the object passed in is going to be modified a reference shape& to avoid lots of copy constructor and destructor calling.

Also when initialising variables in a constructor where possible it is better to use a constructor initialisation list rather than assigning to them in constructor body. Again for classes this is more efficient because it calls a single constructor rather than calling the default constructor and the assignment operator.

So prism might want to look something like

class prism : public shape
{
private:
	string type;
	double depth;
        shape* crosssection

public:
	prism(double d, const square& cs)
        : type("Prism"),
          depth(d)
          crosssection(new square(cs))
        {
        }
};

Note this prism only has a constructor for squares you would have 2 choices, either you write a constructor for every plan subclass of shape but that really is object orientated and is a pain and requires prism to alter every time a new plan shape is added so a better option is …

Banfa 597 Posting Pro Featured Poster

Well the initial thing to do is see if you can write down or actually catty out the process on paper.

It needs to be something like

INITIALISE TRUCK LIST TO EMPTY
CURRENT TRUCK = NEW EMPTY TRUCK
FOR EACH ORDER
    IF THE ORDER WILL FIT IN THE CURRENT TRUCK
        PUT THE ORDER IN THE CURRENT TRUCK
    ELSE
        PUT THE CURRENT TRUCK ON THE TRUCK LIST

        IF THE ORDER IS A SPLIT ORDER
            CALCULATE THE TRUCKS IN THE SPLIT ORDER
            PUT THE SPLIT ORDER TRUCKS ON THE TRUCK LIST
            CURRENT TRUCK = NEW EMPTY TRUCK
        ELSE
            CURRENT TRUCK = NEW EMPTY TRUCK
            PUT THE ORDER IN THE CURRENT TRUCK
        ENDIF
    ENDIF
END FOR EACH

IF THE CURRENT TRUCK IS NOT EMPTY
    PUT THE CURRENT TRUCK ON THE TRUCK LIST
ENDIF

PRINT TRUCK LIST

Also I think your second example is wrong I think trucks 4 and 5 should read

Truck 4 holds: 18 18 for a total of 36
Truck 5 holds: 8

Once you can do the process on paper you stand a better chance of being able to write a program that will do it.

Banfa 597 Posting Pro Featured Poster

Opps missed your comment another reason to post the entire message.

What are you doing trying to open and close an already open stream? out is a working output stream just use it.

Banfa 597 Posting Pro Featured Poster

Did you change the declaration of the function ArrayIntStorage::write as well as the definition?

If you are going to post warning messages post the entire message because the text often contains extra details, the line number can be useful and to anyone not familiar with the Microsoft compiler the number is completely meaningless.

Banfa 597 Posting Pro Featured Poster

Even if you could, which you can't because as someone as pointed out else where ostream might not be a file stream it could be any stream including one without a file name, but even if you could the open in your write function would fail on some operating systems because the file is already open.

I didn't say pass the file name I said pass the output stream.

Your write function would be declared

void ArrayIntStorage::write(ostream& out)
{
    // Code to write to out
}

and you would call it as per line 15

Banfa 597 Posting Pro Featured Poster

Why don't you just pass ostream &out into your write function and write to the provided stream?

Banfa 597 Posting Pro Featured Poster

Calling scanf twice does not mean the user has to enter the data on more than one like. There is no implied reading of a new line character at the end of a scanf call.

scanf just requires whitespace between fields which is space, tab, newline, carriage return but it is not at all fussy about which ones it gets.

Banfa 597 Posting Pro Featured Poster

For what you need to do you can treat the pointer as an array, pointer[0] will work as if pointer was actually an array. Your definitions should look like this

double findMax(double array[], int size)
{
    double max;

    // Code here working on array and size to calculate max

    return max;
}

Delete the array stock1, stock2 and stock3 from the inside of findMax instead do the same operations on array to calculate the maximum value in the array which you can then return.

Banfa 597 Posting Pro Featured Poster

When you instantiate a derived class before calling the constructor of the derived class (TwoDayPackage) it calls the constructor of the parent class (Package). Unless otherwise told it calls the default constructor so

TwoDayPackage::TwoDayPackage(const string &sn, const string &saddress, const string &scity, const string &sstate, int szipcode, const string &rname, const string &raddress, const string &rcity, const string &rstate, int rzipcode, double wt, double shipCost, double f)
{
        setSenderName(sn);
        setSenderAddress (saddress);
        setSenderCity(scity);
        setSenderState(sstate);
        setSenderZIP (szipcode);
        setRecipientName(rname);
        setRecipientAddress(raddress);
        setRecipientCity(rcity);
        setRecipientState(rstate);
        setRecipientZIP(rzipcode);
        setWeight(wt);
        setCostPerOunce(shipCost);
        setFlatfee(f);
}

is equivalent to

TwoDayPackage::TwoDayPackage(const string &sn, const string &saddress, const string &scity, const string &sstate, int szipcode, const string &rname, const string &raddress, const string &rcity, const string &rstate, int rzipcode, double wt, double shipCost, double f)
: Package()
{
        setSenderName(sn);
        setSenderAddress (saddress);
        setSenderCity(scity);
        setSenderState(sstate);
        setSenderZIP (szipcode);
        setRecipientName(rname);
        setRecipientAddress(raddress);
        setRecipientCity(rcity);
        setRecipientState(rstate);
        setRecipientZIP(rzipcode);
        setWeight(wt);
        setCostPerOunce(shipCost);
        setFlatfee(f);
}

However Package has no default constructor, either you have to create a default constructor for the parent class or, and this is what I think you need to do, you put in a specific call to a non default constructor.

Additionally your derived class should not initialise/assign the members of the parent class and the parent class constructor should do that (unless it needs to override some values) so your derived constructor should be

TwoDayPackage::TwoDayPackage(const string &sn, const string &saddress, const string &scity, const string &sstate, int szipcode, const string &rname, const string &raddress, const string &rcity, const string &rstate, int rzipcode, double wt, double shipCost, …
FlynnLags commented: Helpful and solved my issue +0
Banfa 597 Posting Pro Featured Poster

I didn't say clear size before closing the file I said clear size before reading the file.

Come on put some thought into it size has to be the number of records you just read so once if have finished reading a file you don't want to clear size because you will destroy that file. However as you come to read another file you want to start at the beginning of your array again and you want to start counting read records again so that you don't add to what was read from the last file; therefore before reading the file again you need to clear size.

Banfa 597 Posting Pro Featured Poster

Since the ifstream variable ListNumbers is only used inside the iblock of the if statement on line 51 if you declared it on line 52 instead of line 26 then there would be no problem you will have a new file object every time the user choose to open a file.

The only addition is to reset size inside the same if block before reading the file so you fill the array in from the start again.

Banfa 597 Posting Pro Featured Poster

Sorry just saw the code, with your findMin and findMax functions you want to pass and work on a single array, like calcAvg does. The pass back the result as the return value. Don't try and print out in the functions print out in main where the functions are called.

This is generally good advice if your functions that do the calculations also do the printing then you can not reuse them in places that just require the return value. Separate the code that performs tasks from the code than prints results putting them in different functions.

Banfa 597 Posting Pro Featured Poster

You can't pass an actual array to a function but what you can do is pass a pointer to the first element of the array. In many ways a pointer to the first element can be used as an actual array for example

int array[5] = {5, 4, 3, 2, 1};
int* pointer = array;

cout << array[3] << ":" << pointer[3] << endl;

would output "2 : 2".

The important thing to remember is that when you pass an array to a function like this you loose all information about the size of the array so if it is important that the function knows that it has to be passed as well

void function2(int *ptr, int size);

void function1(void)
{
    int array[5] = {5, 4, 3, 2, 1};

    function2(array, sizeof array/sizeof array[0]);

    // array now contains 5, 4, 3, 2, 6
}

void function2(int *ptr, int size)
{
    // Set the last element of the passed array to value 6
    if (size > 0)
    {
        ptr[size-1] = 6;
    }
}
Banfa 597 Posting Pro Featured Poster

Mixing cout and printf to output your results is a mistake.

That 4 being output is the return value of printf, it printed 4 characters.

Get rid of the printf and instead pipe count[c] staight to cout in the normal way. If you want to format the output read up on the io manipulators found in the header <iomanip>

Banfa 597 Posting Pro Featured Poster

Just a couple of notes on how you have implemented your operators. Taking operator+ and operator+= as the examples but it applies to all the other ones too.

Firstly because operator+= does in place addition it can return const Number& rather than Number which is more efficient as there is no object copy on return. Note that operator+ must return Number on the whole to work properly but that may involve copying objects (calling copy constructor and destructor) so is less efficient.

So you have implemented operator+ and implemented operator+= in terms of operator+. That's ok but consider this code

Number result, num;
int i;

// Initialise i and num

num += i;
result = num + i;
result = i + num;

Line 8 will not compile with your object.

Now instead lets pretend that you actually implemented the addition algorithm in operator+= you can write operator+ as member function like this

Number Number::operator+(const Number& number) const
{
    return Number(*this) += number;
}

This form has the advantage that a good optomising compiler can perform the return value optimisation. This optimisation removes the copy of the returned objected in some cases by constructing the return value directly in the place it is being returned to.

Better we can write the operator+ that take differently typed values as helper functions, they don't even need to be friends as they only call the public interface of Number, that allows us to write


       
jonsca commented: Good explanation +4
Banfa 597 Posting Pro Featured Poster

In your first code listing you are explicitly calling the destructors on many of your automatic scope objects, lines 18, 24, 37, 38, but that is completely unnecessary. The compiler automatically adds destructor calls for these objects so you destruct them twice. Since that is likely to involve a delete call it is a receipe for disaster.

In your most recent code listing you delete coefs without zeroing it, test the pointer value of coefs and then deleteit again line 53 - 58.

You should not be deleting an object more than once. Anything else is undefined behaviour (or a memory leak if you don't delete it at all).

Line 56, how can this be anything other than true? If it is false you have a problem because you have 2 polynomial objects using the same data buffer.

On the whole I think your entire class would be simpler if you just used vector<double> coef rather than double* coefsince it would perform the memory management for you.

And finally this for(int i=base.degree;i>=0;i--) is quite a strange way to construct a for loop that is not required to go backwards (minor note and would be an infinite loop if you used unsigned variables).

Normall most people code for loops as for(int i=0;i<base.degree;i++) unless there is a specific need to go backwards through the array. if you used a vector you could just use a reverse iterator if required.

Banfa 597 Posting Pro Featured Poster

You don't use the the output from the stringstream sa to resize your query table.

That is what everyone has been saying. Your query table has a fixed size, that is the number of defined questions, 300 from your first post.

You have 2 options either you select question randomly as you ask them. In this case you need a table with an entry for every question (300) which you use to initially mark all questions as unused and as you ask a question you mark it as used so that if it gets selected again then you can make a new selection rather than asking the same question.

Another option is before asking any questions build a list of the question you will ask that references the complete table of questions.

Ignoring possible problems if the number of questions being asked is a large proportion of the number of available questions those 2 different method both require an additional table that is not the query table.

the query table remains a fixed size it is the ancilary tables whose size may alter depending on the method used to record questions asked/to ask.

dualdigger commented: really helpful suggestions +1
Banfa 597 Posting Pro Featured Poster

Line 46 you have an unmatched { remove it.

Banfa 597 Posting Pro Featured Poster

You will need to change this line as well m_Qtable.resize(m_QNo); as well and in fact you will need to change all instances of m_QNo in your code, both posted and unposted where you have accidentally used the question number rather than the number of available questions as your limit for accessing the question table.

Ancient Dragon commented: Yes, your right. +28
Banfa 597 Posting Pro Featured Poster

Did you seed the random number generator with srand() ?

Banfa 597 Posting Pro Featured Poster
List<String^> Lines = gcnew List<String^>();

Declares an object not a handle and initialises it to an empty list that you gcnew to. However since it is an object and not a handle when you try to assign to it you get an operator= unavailable error because the class List does not implement operator=

If you want to be able to assign new allocated lists to the variable declar it as a handle

List<String^>^ Lines = gcnew List<String^>();
Lines = gcnew List<String^>();
Banfa 597 Posting Pro Featured Poster

Actually this piece of code is not really interested in the sub-class at all. It just needs a Request* in order to queue the request.

It looks like you could use a factory method. A factory method is often a static member of a base class that accepts data and uses the data to create a sub-class of that base but returns a base class pointer.

This keeps the code the knows how and when to create the sub-classes nicely encapsulated in the class your code you then look something like

Request *request=Request::requestFactory(line);

	if (request != NULL)
	{
		requests.enqueue(request);
	}

The factory method looks something like

Request::requestFactory(const QSTring& line)
{
	RequestTypeType requestType = <Decode type From Line>;
	Request* retval = NULL;

	switch(requestType)
	{
	case REQUEST_LOGIN:
		request= new LoginRequest;
		request->tcpMessage=line.toUtf8();
		request->decodeFromTcpMessage();
		break;

	case OTHER_REQUEST_TYPE:
		request= new OtherRequestType;
		request->tcpMessage=line.toUtf8();
		request->decodeFromTcpMessage();
		break;

	...
	}

	return request;
}
metdos commented: Nice, Clean , Informative answer +1
Banfa 597 Posting Pro Featured Poster

The problem is at line 12 loginRequest is created on the stack, that means that it is automatically has the lifetime of the code block it is in which is the if statement.

You take pointers to it and put them on your queue but that doesn't change the fact that the original object is deleted when its lifetime expires.

You need an object whos lifetime is not limited to the code block it is created in. That would be an object allocated from the heap using new.

Change how you use your queue so that the pointers it contains always point to objects allocated using new. When you remove items from the queue remember to delete them once you are finished with them. Possibly look into using std::auto_ptr or a smart pointer or handle class.

Is there any reason you didn't use a std::queue? Is QQueue at QT class?

Banfa 597 Posting Pro Featured Poster

Line 14 coupled with lines 17 and 21 result in a big error. int binary[31]; defines an array of integers with 31 entries in the array indexed as 0 - 30. Certainly not enough to hold a value for each bit in a 32 bit value. for(y = 32; y >= 0; y--) You iterate through the loop 33 times once weach for every value between 32 and 0. Clearly an error when trying to evaluate a 32 bit integer. binary[(32-y)] = x; combined with the previous 2 errors you access array indexes 31 and 32 when y is respectively 1 and 0 both of which are out of bounds array accesses and result in undefined behaviour.

On the algorithm had it been coded correctly. All that raising to powers and reverting to floating point arithmetic is incredibly inefficient not to mention if you were going to do that why not use pow from the standard library rather than write your own.

However the whole affair can be much more simply and efficiently code by using the operators that work directly on bits, this set of operators include

bitwise AND &
bitwise OR |
bitwise NOT ~
bitwise XOR ^
shift left <<
shift right >>

You can use AND to isolate the value of specific bits you can use the shift operators to control which bit you are currently examining.

Banfa 597 Posting Pro Featured Poster

An agregate type is a structure or class or union that holds (possibly) an agregate of serveral members of other types.

In you print function you do not have an agregate type you have a pointer to an aggreagate type. You can get the agregate type by dereferencing the pointer so for a structure type T

T obj;
T* pointer = &obj;  // Always initialise pointers

obj.member;  // Access a member of an agregate type

pointer.member;  // Error pointer is not an agregate type it is a pointer to an agregate type

(*pointer).member;  // OK pointer is derefernced to the agregate type 

pointer->member;   // The -> operator is a shortcut for above syntax
Banfa 597 Posting Pro Featured Poster

operator+= is the in place addition operator. That is it alters the object it is called on, unlike the binary additional operator operator+ which does not alter the object it is called on at all but just returns a new result.

Your impletemtation is not in place, that is it does not alter the object it is called on,it creates a new temporary object temp and alters that.

They way you have written it x += y does not alter x which is what you are seeing. If you wrote z = (x += y) with your current implementation then you would find that x was unaltered by z had the sum of x and y.

You need to re-write your operator+= so that is does an in place calculation, that is so that the object that it is called on is altered to contain the new value. You will not need the variable temp at all in such an implementation.

Banfa 597 Posting Pro Featured Poster

Using ios_base didn't seem to work either. Could it have anything to so with it being a class member function? I'm really just grabbing at straws now!

Using iso_base was not the suggestion the suggestion was use iso_base::ate

Banfa 597 Posting Pro Featured Poster

It sounds like your file is somehow getting truncated before you write out the data or that you are writing to a non-existing file. You need to check out both of these possibilities in full first.

This outFile.write((char*)this, sizeof(A)); is a particularly poor way to write out the class data to file. There are many ways this could go wrong on a slightly more complex class (one with a virtual base or in fact any base, one with a pointer to allocated data). You should write out the individual data members of the class not perform this type of conversion.

Banfa 597 Posting Pro Featured Poster

No you need to alter line 14 of your code listing, it still does not refer to a defined type.

Banfa 597 Posting Pro Featured Poster

This converts->i = 0x1 << 4 | 0x2 << 0; is basically dealing with nibbles (nominally 1/2 a byte, 4 bits) so the value is 0x12 = 18.

However it is normal to deal with bytes not nibbles and that is what ntohl operates on. Not only that but see the l (ell) on the end, that is the function that deals with longs or 4 bytes. It takes 4 bytes in big edian format and converts them to a long value in local machine format which you then output in hex.

What are the first 4 bytes of your array? 0x01, 0x02, 0x03, 0x04
What does it output? 1020304
See the correlation?

I can tell you that it is outputing the correct value given the data you supplied and that your manual calculation is not the calculation that ntohl is doing.

If you want to work on 16bit (short) values you might be better off using ntohs.

Banfa 597 Posting Pro Featured Poster

The problem is (I think) in how you define the structure

typedef struct
{
    int storeNumber;
    char storeName[STORE_NAME_SIZE];
    char phoneNumber[STORE_PHONE_SIZE];
    struct STORE *link;
}STORE;

at line 6, you have no type struct STORE. You have a defined type STORE and an unnamed structure.

if you did this

typedef struct store_t
{
    int storeNumber;
    char storeName[STORE_NAME_SIZE];
    char phoneNumber[STORE_PHONE_SIZE];
    struct STORE *link;
}STORE;

Then you would have a defined type STORE and a structure named store_t or type struct store_t then

struct store_t* ptr1;
STORE* ptr2;

would be equivilent.

Banfa 597 Posting Pro Featured Poster

Trying to simulate the idea is fine but to do that you have to have the correct data in the buffer.

If its a short buffer then use a debugger to set a break point at a point where the buffer is already loaded and copy the data. Alternitively print out the buffer. Alternitively if you have one sniff the data on the wire directly.

What is certain is that the data coming down the wire is not going to be ASCII data like the data you used to initialise your buffer.

Banfa 597 Posting Pro Featured Poster

Of course it prints the ASCII values of 1 and 2, because when you created the string of characters you initialised it with ASCII characters so unsurprisingly that is the values contained.

When you say "I want to read from an array two bytes and print then as a short int" do you mean you want to from an array of 2 bytes containing ASCII values and display them as an integer or do you mean you want to read from an array of 2 bytes containing the binary representation on a short int in big endian format and display the short int value?

In the first case you need to use a function like strtol or aoti and in the second case you have you array initialisation completely wrong you want something like char a[9]={0, 12, 0, 0, 0, 0, 0, 0, 0};

Banfa 597 Posting Pro Featured Poster

Your 2 variables intersection and containment are float arrays that are never initialised.

You then access them by incrementing them, which in itself is undefined behaviour. Since they where not initialised they had a random initial value so the value they have after the increment is also random.

Finally you use the test == 0.0 that is never going to work with a float type. floats and double contain approximations to values the == and != operator are pretty much guaranteed to give you wrong answers a large proportion of the time. Not only that but I see not reason why you have used floats for these variables.

To sum up use an appropriate, non floating point, type for intersection and containment and initialise them before you use them.

Banfa 597 Posting Pro Featured Poster
// Are we painting an edit control?
  if(nCtlColor == CTLCOLOR_EDIT)
  {
    // Yes ...
    // Set the text color to red
    pDC->SetTextColor(RGB(255, 0, 0));
    ...
  }

MSDN also says

To change the background color of a single-line edit control, set the brush handle in both the CTLCOLOR_EDIT and CTLCOLOR_MSGBOX message codes, and call the CDC::SetBkColor function in response to the CTLCOLOR_EDIT code.

so that if statment should be a little more complex (or have further else if clauses.

@mrnobody all of those if statements contain the same block of code so I would not do it like that. If you have direct control of the values IDC_EDIT1 - IDC_EDIT200 arrange the values so that you can implement the condition IDC_EDIT1 <= ID <= IDC_EDIT200 or find some other short cut to answer the question, is it one of my edit boxes.

mitrmkar commented: Yes, a good catch +5
Banfa 597 Posting Pro Featured Poster

In my experience the most likely cause of that type of behaviour is another part of the program writing outside object boundaries. Or writing outside of the boundaries of the object that p2 points at

The second possibility clearly can happen if there is no p2.

For the first possibility when you introduce the variable it alters where a lot of things are stored and the order they are stored in (if only inserting 1 thing somewhere in the order). If a piece of code is writing to somewhere it shouldn't be then this movement of where variables are stored can be the difference between it writing somewhere critical to program operation and somewhere not critical to program operation.

You might want consider using valgrind if you are using Linux, or OS X it can help with detecting that sort of problem.

You might also want to consider splitting you code up into some functions just for readabilities sake if nothing else.

Banfa 597 Posting Pro Featured Poster

If this is QT then the QT documentation specifically says that you must only inherit from QObject once and QObject is a base class of QWidget.

You will have to break the diamond. That might mean having a abstract class that BaseView and ButtonView both inherit from that does no inherit QWidget and not having ButtonView inherit from BaseView.

Banfa 597 Posting Pro Featured Poster

You have a ; at the end of line 18 that should be there.

However once you fix that I would expect all of these types of comparison if (pass = KazzyB) tp cause an error, or certainly not do what you expect.

Banfa 597 Posting Pro Featured Poster

You probably should be overriding the OnCtrlColor handler of the parent window.

Intercepting OnDraw for all your edit boxes sounds tedious.

Banfa 597 Posting Pro Featured Poster
for (int index = 0; index <= limit - 1; index++)

is a bit awkward with the <= limit -1 . Just use the strictly less than comparison to the actual array size (or limit of valid elements), as in

for ( int index = 0; index <  limit ; index++ )

It is more than a bit awkward I have seen program errors introduced by this <= limit -1 construct that are not possible with < limit .

It needs a slightly different situation but imagine that

  1. index has type unsigned rather than int

  2. limit is not a constant it is a variable of the program which has a value >= 0

I found this exact bug in a digital television receiver where presumably the original coder hadn't thought of the possibility of a channel having no audio streams at all although that is perfectly valid.

So if everything is unsigned and limit is 0 then limit - 1 is a very large number, 0xFFFFFFFF on a 32bit system and suddenly you have an infinite loop.

This error does not happen with < limit

vmanes commented: Very interesting observation. +5
Banfa 597 Posting Pro Featured Poster

You can't pass arrays. You may invoke the constructor with an array in the calling code but what gets passed is a pointer. With-in the context of a function declaration int a[] declares a pointer so IntegerSet::IntegerSet( int a[]) and IntegerSet::IntegerSet( int* a) are equivilent. In both cases the parameter a is a pointer.

Just trying to add clarity and this is also the reason Fbody sensibly sugests passing the size of the array along with the pointer.

Of course this is C++ so you should be avoiding arrays altogether an using vectors which solve the knowing what size it is problem, not to mention your constructor would reduce to something like

IntegerSet::IntegerSet(const vector<int>& a)
: set(a)
{
}
Banfa 597 Posting Pro Featured Poster

Line 26 LINKEDLIST *list = malloc(sizeof(list)); Line 34 - 35

STORE *newVacantStore;
newVacantStore = malloc(sizeof(newVacantStore)); newVacantStore = malloc(sizeof(newVacantStore));

In both of these cases you have allocated the size of the pointer not the size of the object pointed to. Pointer is likely to be 4 bytes on a 32bit system (or 8 on a 64bit system).

When you malloc you need to allocate the size of the thing pointed to not the size of the pointer so line 26 should be LINKEDLIST *list = malloc(sizeof *list); or LINKEDLIST *list = malloc(sizeof(LINKEDLIST)); I leave line 35 for you to correct.

Banfa 597 Posting Pro Featured Poster

I didn't say that rand() was inadequate for this purpose, only that that wasn't a very good way of calling it with a link to better ways to use rand()

Banfa 597 Posting Pro Featured Poster

You have given your assignment.
You have given the code you have.

But you have not said what the problem you are having is.
Is it an issue with compiler errors? Then post the errors.
Is it an issue with the program not performing as expect or desired? Then post the desired behaviour and the actual behaviour including all the inputs you gave to the program.


Finally 1 + rand() % 6; is not a good way to generate a random number because of poor randomness in the low bits of the output of some random number generators. Read this.

Banfa 597 Posting Pro Featured Poster

When you declare a data member static there is a single copy of that data member used by all instances of the class so a.n and CDummy::n refer to the same variable.

CDummy::n is incremented every time a class is constructed and decremented when the class is destroyed. So the first cout 7 classes have been created, a, the 5 in the array b and the one newed that c points to, the constructor has been called 7 times and CDummy::n incremented 7 times from 0 to 7.

The c is deleted, the destructor is called and n is decremented so at the next cout CDummy::n has a value of 6 (7 - 1).

PDB1982 commented: Excellent explanation - Thank you! +1
Banfa 597 Posting Pro Featured Poster

start by reading this and relevant pages it links to.

The basic purpose of an assignment operator is to copy all the data from the source object into the current(target) object making sure, if necessary, that buffers are allocated as required if the class uses pointers to allocated data rather than only uses data types (which you class doesn't I believe).

jonsca commented: Rep for this monster thread +3
Banfa 597 Posting Pro Featured Poster

For a managed C++ ref class the assignment operator(operator=) is not generated by default, if you want your class to be assignable you have to create an assignment operator.

Rect2 = Rect;
    Rect2.MoveRect(10, 30);

These lines assign Rect to Rect2 and then move Rect2 just as before, there is no change.