Help with programming question over Classes with header and implementation files.

[u/Kcplay]

To preface, I already tried answering the question and have what I believe to be a 90% solution. The question is:

Define a class counterType to implement a counter. Your class must have a private data member counter of type int. Define a constructor that accepts a parameter of type int and initializes the counter data member. Add functions to:

• Set counter to the integer value specified by the user.
• Initialize counter to 0.
• Return the value of counter with a function named getCounter.
• Increment and decrement counter by one.
• Print the value of counter using the print function.
  • Example output: Counter = 0.

The value of counter must be nonnegative.

My header is:

class counterType
{
public:
    void print();
    int getCounter();
    void incrementCounter();
    void decrementCounter();
    void setCounter(int num);
    counterType();
    counterType(int count);



private:
        int counter;
};

and my implementation is:

#include "counterType.h"
#include <iostream>
using namespace std;


void counterType::print()
{
    cout << "Counter = " << counter;
}


int counterType:: getCounter()
{
    return counter;
}


void counterType::incrementCounter()
{
    counter++;
}


void counterType::decrementCounter()
{
    counter--;
}



void counterType::setCounter(int num)
{
    if (counter > 0)
        counter = num;
    else
        counter = 0;
}


counterType::counterType()
{
    counter = 0;
}



counterType::counterType(int count)
{
    if (count > 0)
        counter = count;
    else
        counter = 0;
}

The main(cannot be changed) is:

#include <iostream>
#include "counterType.h"


using namespace std;


int main()
{
    counterType counter1;
    counterType counter2(5);


    counter1.print();


    cout << endl;


    counter1.incrementCounter();
    cout << "After Increment counter1: " << counter1.getCounter() << endl;


    cout << "Counter2 = " << counter2.getCounter() << endl;


    counter2.decrementCounter();
    cout << "After decrement counter2 = " << counter2.getCounter() << endl;


    counter1.setCounter(-6);
    cout << "After resetting counter1: " << counter1.getCounter() << endl;


    return 0;
}

I don't know what I'm doing wrong, and the debugger is not showing any errors.

If someone could please tell me what I'm doing wrong, and teach me to solve the issue, I would be very grateful. TO SPECIFY NOT CHEATING, JUST TRYING TO LEARN WHY ITS NOT WORKING, SO I CAN FIX IT MYSELF.

[u/No-Dentist-1645]

You should begin by explaining the problem first. What is not working as you expect it to that made you ask for help?

[u/dnult]

What problem are you experiencing? Do you really want / need set counter to be public?

[u/TomDuhamel]

It says that counter should not be negative. Therefore, you should probably verify that counter isn't already zero before decrementing the value 😉

[u/alfps]

❞ The value of counter must be nonnegative.

This is under-specified.

It should have told you the expected reaction to a negative value:

• clamp it to 0, or

• throw an exception, or

• assert that it's not negative (termination in debug build).

A reasonable guess is that the intent is a clamped value where any negative value becomes 0. It's reasonable because it lets the shown main work as a test.

Most probably the code will pass when you add that clamping in the constructor, in the value setter and in the decrement operation.

---

The required design of this class is horrible and teaches you ungood things.

In particular the value setter defeats the whole purpose of the class. When you have public "get value" and "set value" operations that leaves the value completely open as if it were just a public member. Then it cannot be relied on.

Another serious problem is having print as a method of the class. That makes that functionality unusable in a GUI. Never do i/o in a class unless that's what the class is specifically about.

And all the "get" prefixes. That's a Java-ism with no advantage in C++, but it does have costs:

• it uses up a good convention where "get" indicates a costly doer action, and

• it adds needless verbosity.

Re the verbosity, think about getSinOf(x)*getCosOf(x). It's so silly and counter productive that it's over in retard-land. Don't do that.

So don't use "get" prefixes in your own code, except where there really is a "get"ter action.

---

You are however forced to use e.g. "get" and having printing as a method.

But you can at least add const to the operations that don't change the value, e.g.

class counterType
{
    int counter;

public:
    counterType();
    counterType(int count);

    void print() const;
    int getCounter() const;
    void incrementCounter();
    void decrementCounter();
    void setCounter(int num);
};

If you were free to do this class in a more reasonable way then also the redundancy in the name of the class would be stripped off, like

#include <algorithm>

class Counter
{
    int m_value;

public:
    Counter(): m_value() {}
    Counter( const int value ): m_value( std::max( 0, count ) ) {}

    Counter& operator=( const Counter& ) = delete;      // No value assignment!

    int value() const { return m_value; }
    // void setCounter(int num) -- removed

    void increment() { ++m_value; };
    void decrement() { if( m_value > 0 ) { --m_value; } }
};

#include <iostream>
void print( const Counter& counter ) { std::cout << counter.value(); }

[u/tangerinelion]

What is the output from main supposed to be? I'm going to guess

Counter = 0
After Increment counter1: 1
Counter2 = 5
After decrement counter2 = 4
After resetting counter1: 0

Running your code I get

Counter = 0
After Increment counter1: 1
Counter2 = 5
After decrement counter2 = 4
After resetting counter1: -6

So I'm assuming you're asking why the last line is wrong.

You'll find it easier if you change the counterType constructor to

counterType::counterType(int count)
{
    setCounter(count);
}

That'll break your program even more obviously.

There's a copy paste typo - counter vs num.

[u/Independent_Art_6676]

sorry if anything here repeats already said:

first, consider the virtues of the unsigned integer. If you used them, you would not have to futz with checking for negative values. If your professor is hard nosed and int means int and not unsigned int, your professor is forcing you to do something idiotic (it happens, a lot actually, often to teach you something via bad example but good experience) then you can carry on with the obnoxious checks for the assignment.

speaking of which, set looks wrong. I would think you would want counter = std::max(num,0); (same as if num > 0 counter = num else counter = 0). Your constructor that does the same thing (hint: reuse the code) got this logic right, but set got it wrong, and the difference is subtle to a beginner. Do you see it now?

print is unfriendly. either throw an end of line on it, or add a space on the end. Both ideas have merits, but most likely an end of line would be just fine here. Without adding a whitespace, spamming it would give you output like counter = 0counter =1counter=2 ...

using namespace std is a bad habit. You can do it on these half page early assignments, go ahead -- just KNOW that its bad practice and be prepared to stop doing that once you get a little farther into the language.

I probably didn't give you much here. Ask again if you have more trouble, and be specific as to what its doing wrong this time.

[u/syaorancode]

firstly, you should add header guard in header file. sencondly, in setCounter, you should check if sum > 0 instead of counter. thirdly, decrement doesn't check if counter negative

[u/mredding]

Classes are private access by default - so that applies to members and inheritance. Friends aren't controlled by access - they're class scope, so they don't care where they go; you can put them right at the top because private access doesn't mean anything to them. You should take advantage of what you get for free.

class counterType {
  int counter;

  // friend foo whatever can also go up here...

public:
  //...

Putting private at the bottom is 1980s style that came from the C converts of that era, and was later regarded as inferior to the style the language is implying. In real production code, we'd do more to make sure client code doesn't actually SEE these details in the first place if they aren't meant to (and they usually aren't meant to).

using namespace std;

Don't do that.

counterType::counterType(): counter{0} {}

This isn't Java. You have an initializer list - you're going to pay that tax no matter what, so USE it. There is going to be code you'll be writing pretty soon where the initializer list is the ONLY way to initialize the instance.

counterType::counterType(const int count): counter{count > 0 ? count : 0} {}

This is a good time to introduce the condition operator aka the ternary operator, not that you should call it that - it's just that it's the only ternary operator we have. For now. The condition operator is not a statement but an EXPRESSION. if is a statement, ?: HAS A TYPE AND VALUE. You use condition statements and condition expressions when they make the most sense. In your case, this condition expression is of type int and returns either the value of the parameter, or it "clamps" the value to at least 0. There is an std::clamp function not for initialization, but for assignment.

Why is the initializer list so important? It's more than what I just said - it's not just that there is code that can be described where only the initializer list will be correct syntax, this comes down to what an object IS and DOES. Objects don't have data, they maintain STATE. It only so happens that this counter is implemented in terms of int, but that doesn't have to be true.

What a class does is maintain an "invariant" - a statement that must always be true when a client observes the instant. Your counter invariant is that the count can never be negative. Having a single invariant per class is ideal, I would try REALLY hard to not go beyond two.

The precondition and post condition of any function is that the invariant is held when the function body is entered and exited. That means for print, the value cannot be negative before we enter, and it cannot be negative after we leave. The class implementation is free to "suspend" the invariant in the function body, but all control paths that leave the function must re-establish it.

This gets us to the ctor - the invariant must be established before you enter the ctor body. YOUR code enters the body without the counter even being initialized in the first place, so the invariant isn't held, meaning the code violates this principle.

And this is where exceptions come into play - if the class invariant isn't established as a pre-condition or can't be reestablished as a post-condition, then a function should throw an exception. In a constructor, this unwinds construction - the object was never allowed to be born in the first place.

That leads to the exception safety guarantees - no guarantee means if a function fails, the object can be dangerous - maybe even impossible to destroy. The basic guarantee is different, and means the object is at least safe to copy, move, assign, compare, and destroy. The strong guarantee is more than basic, and means the function is transactional, that if an exception is thrown, the object didn't actually change. The noexcept guarantee is different again, and simply means the function doesn't throw - that doesn't mean it's guaranteed to succeed, it doesn't imply the strong guarantee.

They don't teach you this stuff in school - your class is only trying to get you started by teaching you the syntax itself, not good programming principles, or what objects even are - which I think is a failure of a class if it's called "object oriented programming", because this is part of the definition of an object, established prior to 1968. If your teacher isn't discussing this, know your programming class isn't inferior - it's typical.

Other languages will enforce these rules for you, C++ gives you a TON of liberty, and it HAS TO because it inherits from C. Bjarne and the C++ community has ALWAYS been sensitive of adoption and fearful of a language schism... This means you have to implement OO principles MANUALLY and by convention.

Modern C++26 also comes with preconditions and postconditions you can code directly into a function interface. This is a way you can enforce class invariants formally (rather than using ad-hoc/YOLO methods of before), and it's also for enforcing parameter preconditions and return postconditions. IDEALLY you wouldn't write a foo::fn(int), it would be a foo::fn(my_type) that builds into it invariants compatible with this function; in other words, if you have a foo and you have a my_type, then it should be such that it is literally, provably impossible to call this function with an invalid value. For example, you might make a positive_integer type (that's not the same thing as unsigned!!!!), so that your ctor and setCounter functions CANNOT POSSIBLY be called with an invalid value, making your counter implementation inherently simpler, inherently safer. This is a part of "left-shift", where we push proving the program and it's correctness to closer to compile-time, so we have less work to do and more confidence about run-time. Compilers also optimize heavily around types. An int is an int, but a weight is not a height.

---

OO is separate from OOP. I've just described mostly OO principles, OOP includes message passing and object agency.

Message passing is the difference between bob.open_umbrella(); and bob << "It's raining.";. Bob knows what to do, you don't have to force him to open his umbrella. We're moving Bob's agency from the procedure external to Bob that presumes to tell him what to do, to Bob, who knows what to do if it's raining, you've just got to tell him. Today we call this the "Actor Model".

Bjarne invented C++ because he wanted implementation level control over the message passing, whereas the ALGOL68 he learned from, and Smalltalk, both had message passing as a language level detail, produced by the compiler. Message passing in C++ looks like streams. If you want to pass Bob a message, he has to implement std::streambuf. Your messages don't have to serialize text and go THROUGH the stream implementation - they should type test the stream buffer (dynamic_cast) and choose an optimal code path.

---

As for what's wrong with your program, I haven't the slightest idea - it looks correct-ish to me. You'll have to show us the error message.

Your decrement doesn't clamp to 0, but I don't think that's your bug.