0

This program uses a structure to store the following data on a company division:

Division Name(such as East, West, North, or South)

Quarter(1, 2, 3, or 4)

Quarterly Sales

The user should be asked for the four quarters' sales figures for the East, West, North, and South divisions, and the data should be stored in the structure. This is my code so far, I'm not understand the concept properly:

#include "stdafx.h"
#include <iostream>
#include <fstream>

using namespace std;

struct Data{

    char *name;
    int quarter[4];
    int sales;

};

int _tmain(int argc, _TCHAR* argv[])
{
    Data division[4];
    int count;

    division[0].name = "East";
    division[1].name = "West";
    division[2].name = "North";
    division[3].name = "South";

    for(count = 0; count < 4; count++){

        cout << "Please enter the sales for the " << division[count].name << " for: " << endl;
        cout << "Quarter: " << count + 1 << ": " << endl;
        cin >> division[count].quarter[count].sales;


    }

    cin.get();
    return 0;
}

It seems my for loop is not working at all, something is wrong with my cin statement, because `division is showing a red underline.

AVH
  • 11,349
  • 4
  • 34
  • 43
user566094
  • 7
  • 1
  • 6
  • my for loop is not working at all, something is wrong with my cin statement, because 'division' is showing a red underline – user566094 Dec 15 '11 at 10:46
  • This looks like homework, so I tagged it as such. – AVH Dec 15 '11 at 10:50
  • aha. the division is showing a red line. I can - sort of - _guess_ what that means. But really, you should try defining what that means. It will help you get to solving these things on your own. A red line is not magic. It means something: the compiler (intellisense engine, actually) is telling you there is a (syntax) error in that spot. – sehe Dec 15 '11 at 10:52
  • Since there are only 4 divisions to choose from i think it would be better implemented with a enum. And quarter[] is an array so quarter[count].sales it's nonsense. Again if you want quarter sales, you could write them in the array where the array index would be the quarter id( cin >> division[count].quarter[count];) – 6D65 Dec 15 '11 at 10:57
  • 2
    You are using C++, try using the vector rather than an array. – DumbCoder Dec 15 '11 at 11:40
  • 2
    Serious advice: Get a good C++ book, your program is far from being wellformed: `tmain` is non-standard; assigning string-literals to `char*` is not allowed; `_TCHAR` is non-standard; `cin.get()` is not needed and prevents a dead program from dying; `stdafx.h` is non-standard; use `++count` instead of `count++`; you are using `endl` excessively; you do _not_ need `#include ` / So many mistakes that giving advice on your concept would be harmful as you would in turn manifest your mistakes even more! Get a good C++ book first and build the fundament **before** the building! – Sebastian Mach Dec 15 '11 at 11:51
  • @user566094: does it compile? – PlasmaHH Dec 15 '11 at 12:18
  • @phresnel To me, it seems the most common syntax for incrementing variables in a for loop is the `i++` style. Just out of curiosity, why do you advocate writing `++count`? In this context, the prefix and postfix increment operators are functionally identical, and post increments are the "standard way of doing it". – Alderath Dec 15 '11 at 12:28
  • @PlasmaHH No need to remark on that when he stated in the question that it doesn't compile... – Alderath Dec 15 '11 at 12:31
  • 1
    @Alderath: You often waste performance (think of more complex iterators) with post-[in/de]crement, while with pre-[in/de]crement you never do. I don't get why post-decrement, which conceptually creates a copy of the incremented value that is then returned to the caller, should be preferred. It may make no difference here, but why not make a habit of the never-wasteful one? See it like this: `i++` says `save a copy of i, increment i, give me the copy`, `++i` says `increment i, give me the value`. Advocates of pre-increment: Meyers, Koenig, C++FAQ, Vandevoorde, Josuttis, libstdc++, many more. – Sebastian Mach Dec 15 '11 at 13:09
  • @phresnel That's quite interesting. I would think that any half decent compiler would be able to optimize that away. Isn't that the case? Seems like it'd be a pretty darn simple optimization. Isn't that as silly as saying that you should left-shift instead of multiplying by two? Multiplying by two is preferable, since it is more readable, and the compiler will probably change the multiplication into a bit shift operation anyways. – Alderath Dec 16 '11 at 12:50
  • @Alderath: That's not the same. The semantics of pre-increment and post-increment are different. The one says "give me the value before ++", the other says "give me the value after ++", so comparing this with shift/multiply is not accurate. However: The performance for builtin types is certainly the same, true. But for types the compiler has less knowledge about (e.g. because you define `Foo::operator++` in a seperate source-file), the compiler cannot deduce equivalence (except with non-trivial global/whole-program optimizations), and if it can't guarantee safety, it doesn't risk it. – Sebastian Mach Dec 16 '11 at 12:58
  • @Alderath: Some references: http://stackoverflow.com/questions/1303899/performance-difference-between-iterator-and-iterator , http://llvm.org/releases/1.5/docs/CodingStandards.html#hl_preincrement , http://www.gotw.ca/gotw/002.htm – Sebastian Mach Dec 16 '11 at 13:03

5 Answers5

2

Replace

cin >> division[count].quarter[count].sales;

with

cin >> division[count].sales;

But your application structure is... fishy at least. What's the point of the quarter array?

Luchian Grigore
  • 253,575
  • 64
  • 457
  • 625
0

In your original you are going to overflow your quarter array... This is dangerous and undefined behaviour and can lead to memory corruption that will pop up at a later time and mess your program up. How are you going to overflow? Well, when your count value goes to "3" you are going to address your quarter array with quarter[3+1] = quarter[4]. This is ONE more than the array holds (from 0 -> 3) and this memory address will get written to. What is in that address? We don't know.

It looks like you should have a nested for loop for the quarters:

for(int div_i = 0; div_i < 4; ++div_i)
{
    for(int qt_i; st_i < 4; ++qt_i)
    {
        cout << "Please enter the sales for the " << division[div_i].name << " for: " << endl;
        cout << "Quarter: " << qt_i << ": " << endl;
        cin >> division[div_i].quarter[qt_i].sales;
    }
}

That would leave you with the individual quarter sales for each division.


Also you should use a const uint8 for your array sizes:

const unsigned char _DIVISION_COUNT = 4;
const unsigned char _QUARTER_COUNT = 4;

// and your loops become:
for( int count = 0; count < _DIVISION_COUNT; ++count)...

Makes it clear to someone who would use/maintain your code after you what you are doing.

Dennis
  • 3,683
  • 1
  • 21
  • 43
0

You've got a couple of problems with the structure you defined.

First, what is the type of the sales data. You're specifying integers, but is the sales really a decimal number (many cash examples use doubles for money representations)?

Second, I am a little suspicious of the problem statement. Your structure example involves an array, but the problem statement halfway suggests that each structure will have a single indicator of what quarter it represents. This would make the structures more like database records.

Here are two possible approaches:

typedef double money_t;

struct sales_datum {
    std::string division; // North, East, South, West
    int quarter; // 1, 2, 3, 4
    money_t sales;
};

The above approach seems to comply with the problem statement a little more. Each of these structures would hold one piece of sales data - for a particular division for a particular quarter.

Alternatively,

struct sales_datum {
    std::string division; // North, East, South, West
    money_t sales_for_quarter[4]; 
};

This seems to be more like what you were trying in your original post. The struct now holds sales data for one division for an entire year (four quarters).

Using the second type will tend to make your code go in certain directions - it's easier to think about filling in the entire structure before moving on to another one, so you'll want to collect all the data for a year for one division before moving on to the next.

Austin Hastings
  • 617
  • 4
  • 13
-1

A structure is a very simple definition of a class. Think of it as a class without any methods, just instance variables.

What you're doing there is asking, for each division, what the sales are for each quarter.

So in every division you have 4 different sales, pertaining to each quarter. Those are saved in your structure.

swiftcode
  • 3,039
  • 9
  • 39
  • 64
-1

You are using the same variable to loop over both divisions & quarters (both times count).

AVH
  • 11,349
  • 4
  • 34
  • 43