If she floats then she is not a witch like we thought

Question

Continuing my C++ saga, this is the third project for my CS1 class:

Buoyancy is the ability of an object to float. Archimedes' principle states that the buoyant force is equal to the weight of the fluid that is displaced by the submerged object. The buoyant force can be computed by

$$ F_b = V \cdot y $$

where \$ F_b \$ is the buoyant force. \$ V \$ is the volume of the submerged object, and \$ y \$ is the specific weight of the fluid. If \$ F_b \$ is greater than or equal to the weight of the object, then it will float, otherwise it will sink.

Write a program that inputs the weight (in pounds) and radius (in feet) of a sphere and outputs whether the sphere will sink or float in water. Use \$ y = 62.4 \dfrac{\text{lb}}{\text{ft}^3} \$ as the specific weight of water. The volume of a sphere is computed by \$ \left( \dfrac{4}{3} \right) \pi r^3\$.

Additionally, I must have a loop in this program that allows the user to run it as many times as they want to. I have to ask the user each time whether they want to continue.

It is also supposed to be fully commented, which I think I have done within reason.

buoyancy.cpp:

/**
 * @file buoyancy.cpp
 * @brief Calculates if a sphere will sink or float in water given the weight and radius
 * @author syb0rg
 * @date 9/12/14
 */

#include <iostream>
#include <limits>
#include <math.h>

/**
 * Sphere
 * @var weight The weight of the sphere
 * @var radius The height of the sphere
 */
struct Sphere
{
    double weight = 0;
    double radius = 0;
};

/**
 * Resets the command line so we can input data again, and signals user to re-enter proper data
 */
void resetConsole()
{
    std::cin.clear(); // clear the error flag that was set so that future I/O operations will work correctly
    std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n'); // skips to the next newline
    std::cout << "Invalid input.  Please enter a positive number: ";
}

int main()
{
    constexpr double waterBuoyancy = 62.4;
    Sphere obj;
    char again = '\0';
    do
    {
        // get input for height, re-read input if not a positive number
        std::cout << "Enter the weight in pounds: ";
        while(!(std::cin >> obj.weight) || obj.weight < 0) resetConsole();

        // get input for radius, re-read input if not a positive number
        std::cout << "Enter the radius in inches: ";
        while(!(std::cin >> obj.radius) || obj.radius < 0) resetConsole();

        double volume = ((4/3) * M_PI * pow(obj.radius, 3));
        double bouyantForce = volume * waterBuoyancy;

        if (obj.weight <= bouyantForce) std::cout << "The object will float." << std::endl;
        else std::cout << "The object will sink." << std::endl;

        std::cout << "Run the program again (y/N): ";
        std::cin.get();  // absorb newline character from previous input
        std::cin.get(again);
        again = tolower(again);
        if ('\n' == again) again = 'n';
    } while (again == 'y');
}

Answer 1

There are several small things that you can improve:

• double weight = 0; works fine, but double weight = 0.0; would be more pedantic since weight is a double.

• By the way, the following line is a good example to illustrate the benefits of literals pedantry:

  double volume = ((4/3) * M_PI * pow(obj.radius, 3));
  

  Here, 4/3 performs an integer division and not a floating point one; that expression will yield 1 instead of 1.333333333 (I doubt that you want it to yield 1). You should change it to 4.0/3.0 to get the desired result.

• That said, there is another problem with the aforementioned line: M_PI is not standard C++. It is not standard C either. It is a standard POSIX addition to <math.h>. You should rewrite your own constants or use Boost ones for examples if you want your code to be portable.

• Speaking of <math.h>... You are using C++, therefore you should use the C++ standard library headers too and not the C ones:

  #include <cmath>
  

• And you notably forgot to include the header <ctype.h> for std::tolower.

• That said, it is good practice in C++ to fully qualify the names of the functions from the standard library: you should write std::tolower (and thus actually include <cctype> instead of <ctype.h>). It's not really longer and it may help to prevent name clashes (I doubt that you will have problems with tolower, but it can be worse with some more common names).

Answer 2

Bugs

You calculate the volume of the sphere incorrectly!

Style

• You misspelled the variable bouyantForce.
• The constant waterBuoyancy is a misnomer. It should be waterDensity.
• There's not much point to struct Sphere. You haven't defined any methods in it, nor do you use it to facilitate parameter passing. You might as well have two independent variables named weight and radius. (You could also take the other approach, which is to make it a class.)
• Instead of (or in addition to) defining resetConsole(), you could define a askDouble(promptString) function instead. That would reduce repetition and make your main() easier to follow.

Answer 3

char again = '\0';

No need for setting a value. You overwrite it before using anyway.

do 
{
    // ...
    if ('\n' == again) again = 'n';
} while (again == 'y');

The if here seems pointless. If again is not 'y', you will exit the loop anyway, so setting again = 'n' seems pointless, together with the if.

Although you created a struct for Sphere, it doesn't have much purpose in this code. The Sphere is not doing much for you. You set its weight and radius, and then using those fields directly to calculate the volume. It would make sense to move the calculation of the volume inside Sphere. That way, the object would have more purpose: users wouldn't have to worry about how the volume is calculated, the logic would be encapsulated inside.

You can imagine different kind of objects implementing an appropriate volume method. You could have a collection of such objects, and not need to know how to calculate their volume from their internal properties, but simply ask their volume method.

Answer 4

Regarding the struct, you can just keep it in main() if you'll just be creating an instance and possibly passing an instance to another function. Keep in mind that structs are public by default in C++.

Adding on to what @200_success has mentioned, you can add more to it, such as an initializer list to give an instance starting values, operator>> overload to input directly into fields, and operator<< overload to display fields of an instance directly. You may even find operator>> to be more streamline than accessing the individual fields (you can have the overload take both input values at once).

Essentially, you can utilize these things, along with @janos' volume calculation idea, to make Sphere a more useful structure. If something belongs to a Sphere, keep it within the structure so that the user can let Sphere handle it.

The final struct Sphere can look something like this:

struct Sphere
{
    double weight;
    double radius;

    Sphere(double weight, double radius)
        : weight(weight)
        , radius(radius)
    {}
};

std::istream& operator>>(std::istream& in, Sphere& obj)
{
    return in >> obj.weight >> obj.radius;
}

std::ostream& operator<<(std::ostream& out, Sphere const& obj)
{
    out << "Weight: " << obj.weight;
    out << "\nRadius: " << obj.radius;
    return out;
}

(The respective overloads require <istream> and <ostream>, not <iostream>.)

Note that the overloads are defined outside the struct. While it is still okay to have them inside as friend functions, Scott Meyers recommends non-member non-friend whenever possible.

This doesn't do any input validation (operator>> could handle the validation), but you can implement that however you'd like. Even without this extra code, the struct may fit better in a separate file, but with this simple implementation, it may be a little overkill. This should at least give you some ideas.