7
\$\begingroup\$

I am new to C++ and programming in general. After reading through my first book, I decided to take on some programming exercises and test what I learned. The exercise is phrased as follows:

Find Cost of Tile to Cover W x H Floor - Calculate the total cost of tile it would take to cover a floor plan of width and height, using a cost entered by the user.

This is the code I wrote for it:

#include <iostream>

using namespace std;

int main() {

    double StartSquare, cost, width, height;    // Starting square meters and cost. Width and height of floor.

    cout << "Enter value of 1 pack of tiles: ";
    cin >> cost;

    cout << "Now enter the square metres covered by the pack: ";
    cin >> StartSquare;

    cout << "What is the width of your room? ";
    cin >> width;
    cout << "Now enter the height:\n";
    cin >> height;

    cout << "You need to cover a total of " << width * height << " square metres.\n";
    cout << "The total cost will be: " << (width * height) * cost << endl;

    return 0;
}

It does what I need it to, I just want to know whether it can be improved upon. This also brings me to question of: How do I self-review my own code? I don't want to have to constantly be asking a review of my code and rather self-assess it myself.

\$\endgroup\$

migrated from stackoverflow.com Jul 15 '15 at 7:13

This question came from our site for professional and enthusiast programmers.

  • 4
    \$\begingroup\$ To address how you can review your own code: you read a lot of good code, and you constantly compare how it is better than your own. What constitutes good code? You'll know it when you see it, after having read a lot of other code. Do read books about design patterns. Do not learn from books like How to learn PHP in 48 Hours. Post your code to Code Review and learn from others. (Don't worry about asking others; the whole point of that site is reviewing code!) Then you start writing reviews of other users' code, and see what feedback you get in comments and votes. \$\endgroup\$ – 200_success Jul 14 '15 at 22:48
  • 1
    \$\begingroup\$ So, basically, you say that you don't want your code reviewed, but this is actually a Code Review question in disguise. \$\endgroup\$ – 200_success Jul 14 '15 at 22:49
  • \$\begingroup\$ @200_success I took a look at the site you mentioned, and had I done a bit more searching I would have posted there. Really apologise for posting here. I would never learn from books that claim that sort of thing. Regardless, thank you for taking the time to address my question :). \$\endgroup\$ – H Ozdogus Jul 14 '15 at 23:02
6
\$\begingroup\$

Your calculation was wrong: it did not make use of StartSquare at all. The bug might have been avoided with better naming: cost is too ambiguous. (Better naming would also be more valuable than the comment.)

using namespace std; is considered bad practice, since it defeats the benefits of namespaces. It will work just fine for a small program like this, but you might as well develop good habits.

A key to good programming is recognizing patterns. Here, you're doing the same basic task four times: print a string to std::cout, and read a double to std::cin. That deserves to be a function.

#include <iostream>

double askDouble(const char *prompt) {
    double answer;
    std::cout << prompt;
    std::cin >> answer;
    return answer;
}

int main() {
    double costPerPack = askDouble("Enter value of 1 pack of tiles: ");
    double areaPerPack = askDouble("Now enter the square metres covered by the pack: ");
    double width  = askDouble("Enter the width of your room: ");
    double height = askDouble("Now enter the height: ");

    double area = width * height;
    std::cout << "You need to cover a total of " << area << " square metres.\n"
                 "The total cost will be: " << area / areaPerPack * costPerPack
              << std::endl;
}

In C and C++, many functions can fail, but it's easy to forget to handle the possibility of failure.1 In your program, what could possibly go wrong?

What if you enter non-numeric input at one of the prompts? One data-entry error on the first question will cause an immediate failure for all of the questions. That's unexpected behaviour. The fix involves clearing the input buffer and asking again. Here, you'll see how the function we created earlier pays off!

(exit(1) is not a good practice. Usual practices are to either return an error code or throw an exception. But I'll keep it simple for this illustration by just exiting.)

#include <cstdlib>
#include <iostream>

double askDouble(const char *prompt) {
    while (!std::cin.eof()) {
        std::cout << prompt;
        double answer;
        if (std::cin >> answer) {
            return answer;
        }
        std::cin.clear();
        std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
    }
    exit(1);
}

1 In my experience, adding proper error handling to a program takes approximately as much time as implementing it in the first place without the error handling.

\$\endgroup\$
  • \$\begingroup\$ Extremely informative. I appreciate the time you took to help. With the namespace, I was reading about that after I posted this and now try to make a note not to use it. I'm assuming that a proper way to handle the exception is through try, throw and catch. \$\endgroup\$ – H Ozdogus Jul 15 '15 at 12:26
6
\$\begingroup\$

One of the most powerful techniques you will develop is called 'separation of concerns'. What this means when it boils down is that your program can be split into discrete logical units. Ideally one module (or in the case of c++, class) will do exactly one job without depending on any other.

This has a number of benefits in that the parts can be re-used without needing any knowledge of which parts of the program are using them.

here is your program again, split into essentially three sections, or concerns. Hopefully you will see that although there are more lines of code, the logic in the main() function is easier to follow. Also you can hopefully see that the "input_data" class is an entirely autonomous entity, which could be re-used to gather data for other parts of a bigger program:

I also fixed the small logic error in your calculation, which was easier to see when the calculations were separated from the input/output code.

#include <iostream>

using namespace std;

double area(double width, double height)
{
    return width * height;
}

double cost_to_cover(double area, double area_per_pack, double cost_per_pack)
{
    double packs_needed = area / area_per_pack;
    double cost = packs_needed * cost_per_pack;

    return cost;
}

struct input_data {
    double area_per_pack, cost_per_pack, width, height;    // Starting square meters and cost. Width and height of floor.

    void gather() {
        cout << "Enter value of 1 pack of tiles: ";
        cin >> cost_per_pack;

        cout << "Now enter the square metres covered by the pack: ";
        cin >> area_per_pack;

        cout << "What is the width of your room? ";
        cin >> width;
        cout << "Now enter the height:\n";
        cin >> height;
    }
};

int main() {

    // concern 1 - gather data
    input_data input;
    input.gather();

    // concern 2 - perform calculations
    double room_area = area(input.width, input.height);
    double cover_cost = cost_to_cover(room_area, input.area_per_pack, input.cost_per_pack);

    /// concern 3 - output the result
    cout << "You need to cover a total of " << room_area << " square metres.\n";
    cout << "The total cost will be: " << cover_cost << endl;

    return 0;
}
\$\endgroup\$
  • \$\begingroup\$ Comments are not for extended discussion; this conversation has been moved to chat. \$\endgroup\$ – Mathieu Guindon Jul 15 '15 at 18:13

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.