Question of programming style...

- inherit different classes,
If you use normal functions, you can already do this. Normal functions can call and reuse each other.

- Encapsulate functions and variables
This isn't what "encapsulation" means. You're not encapsulating anything because you're not presenting any kind of interface to the outside world. What you're doing is abusing class members to avoid passing data among the functions. Basically you're making the code more difficult to read because it's not immediately clear what data the function may modify or read.
Compare:
1
2
3
4
5
6
7
8
9
10
11
12
13
class Foo{
    int a, b, c;

    void f();
    void g();
public:
    Foo(){
        a = 0;
        b = 0;
        f();
        g();
    }
};
1
2
3
4
5
6
7
int f(int a, int b);
void g(int c);

void Foo(){
    int a = 0, b = 0;
    g(f(a, b));
}


- Use virtual function calls.
You can't safely make virtual calls with the object being constructed, because the virtual table is in a state of partial initialization during construction.

Don't do this.
* It has no actual advantages that can't be obtained by other means.
* It's unconventional. Other people may be confused by this kind of code because it's not expected that something that looks like a function call is actually an object construction, nor is it expected that something that looks like a class is actually a function.
* It's an abuse of classes. A class is something that is, not something that is [i]done[/I].
... helper class who's only purpose is to do a single batch of calculations and return some data


If you are creating a class whose only purpose is to compute, then you probably don't need the class. A class is supposed to represent something tangible, not to be used as a container for names. We have namespaces for that.

I don't think anything is wrong with the discarded-value thing MyClass().DoSomething( a, b ), and in fact I'd argue that short method chains are sometimes slightly preferable to polluting the scope.

Method chains sometimes require looking up what every invocation's return type is, whereas if you make it explicit you don't have to keep all of it in your head. It's a similar problem to what arises when you use auto everywhere.

Last edited on
- many functions can be declared private and used to aid in the calculations
You can do this with static functions, although there is not much point.

Member variables can be used, such that the private functions don't have to have a whole list of parameters passed to them
This is a disadvantage, as I've said.

which can get problematic when the parameter count becomes rather large
If you need to pass around a large number of parameters, that points to an issue in the design. Bad style is not a solution to bad design.

By creating "helper" or "calculator" classes, the data containers can be meaner, leaner, and much easier to manage.
Why are normal functions not suitable for this? For example, std::vector doesn't have a sort() member, but there is an std::sort() function. There is no std::sort class.


Do what you want, but if you work in a team that does code reviews, using classes like this will get your code rejected.
Last edited on
e.g. If you are doing a mathematical simulation and it requires 10 parameters, then no amount of design changes will change that fact, the calculation needs 10 parameters.
This is not an example. You've just stated a property of an example you've not provided.

3) Static variables
I don't know why this is even listed as an alternative.

4) Virtual functions
? I don't see how virtual functions are an alternative to passing many parameters.
 
p->polymorphic_call(a, b, c, d, e, f, g, h, i, j);
Yay...

2) have a class/struct containing the parameters
- not bad, but having to dereference the values inside each sub-function gets messy as well
e.g.
void SubFunction( MyParameters *params )
{
value = params->a + params->b....
}
In a correct design you would have something like this, except the data the function operates on would not be passed around in an impromptu struct, but it would be part of an object to begin with.
To begin with, a function that does something with a lot of disparate data is something very unusual.
Consider this:
1
2
3
4
5
6
7
8
9
10
void Mechanic::fix_engine(Vehicle *vehicle){
    EnginePart *part;
    while ((part = this->locate_fault(vehicle->get_engine()))){
        if (this->attempt_to_fix(part))
            continue;
        EnginePart *new_part = manufacturer->order_replacement(part);
        vehicle->get_engine()->replace(part, new_part);
        delete part;
    }
}
There's no reason for fix_engine() to accept each EnginePart as a separate parameter. You'll never want to fix_engine() on different parts of different vehicles; that would be senseless.
Additionally, notice how the various functions are passed only the data they actually work on. It's not workshop->locate_fault(workshop), it's mechanic->locate_fault(engine).

If you can give a concrete example where such a redesign is unfeasible, I will concede that your abuse of classes is acceptable in such an instance.
Isn't the OP basically describing a functor?
Helios,

I'm gonna stop you for a moment, and say that I believe we have a lack of communication. Either I did not explain myself well, or you did not understand, or a combination of both.

The example you just posted IS what I'm talking about. The "helper class" that I am referring to is the "Mechanic" in your example. The only difference might be that you may need to tell the mechanic several different things, like what part to fix and how to fix it, hence that's where the parameters start creeping in.

The initial point about doing all the work inside the constructor was pointed out as being faulty, due to the virtual table not being fully constructed. I thank you for pointing out this oversight.

Other than that, it's a simple matter of style. The only point beyond that was that if the mechanic is only going to do one thing, should it be like

------------------------------

Mechanic mech;
mech.fix_engine( vehicle )

v.s.

Mechanic( vehicle ); // which would be cool if not for the problem with the virtual tables

vs.

Mechanic().fix_engine( vehicle )

------------------------------

That's it really. That was the whole point. The second case is unusual ( and turns out to be for a good reason) and that's why I made the posting. Everything else is business as usual.

So, I think you got fixated on the original point about using the constructor ( admittedly an error ), and got the impression that the whole thing was an abuse of classes, which it obviously is not. It's a simple matter of one line vs. two. Again, a point of style.

As mbozzi pointed out, there are many cases where a method chain is preferable to avoid "polluting the scope" it was put.

I did start off this posting with the caveat that my nomenclature is a bit weak, and so I think that you got the wrong idea when I said "helper class". Again, when I said helper class, I meant something like your Mechanic example which is a "helper class" to the vehicle.

So, in conclusion,
- Thank you for pointing out the error with the constructor and the virtual table.
- In the future, you might consider using less aggressive language ( i.e. "your abuse of classes" ) since it is first of all inaccurate, and second of all, makes you come off sort of like a troll. I'm sure you are a perfectly fine sort of chap, and you didn't mean to be like that, that's just how it came off.

No, I understood you perfectly well.

As mbozzi pointed out, there are many cases where a method chain is preferable to avoid "polluting the scope" it was put.
And as I pointed out, there are ways to do this that don't involve classes. Besides static functions that I've already mentioned, you also have namespaces.

Again, when I said helper class, I meant something like your Mechanic example which is a "helper class" to the vehicle.
You misunderstood the example, that's understandable.

Like I said, a class is something that is, not something that is done. A Mechanic is not a FixEngine. A Mechanic may have an interface with several functions attached to it. In my example I use fix_engine() and locate_fault(). It may also have other functions, like deal_with_customer(), take_a_break(), leave_for_the_day(), etc. These functions may change the state of the Mechanic in various ways (e.g. fix_engine() may reduce some stamina value, while leave_for_the_day() may increase it).
The point of a class is twofold: 1) it maintains an internal state that may be modified during the lifetime of its instances, and 2) it presents an interface to the outside to perform some useful function, possibly involving the internal state. Your classes only make use of #2, since the state of the object is gone after the class' main function returns. It's unclear what happens in this case:
1
2
3
Mechanic mech;
mech.fix_engine(vehicle);
mech.fix_engine(vehicle2);
If the first call leaves mech in an invalid state, that's obviously bad. If fix_engine() is the only function in the class and it doesn't change the state of mech, it's questionable whether Mechanic really needs to be a class, but it is acceptable.

Not everyone who disagrees with you is a troll. If you can't take criticism, you should stay off the Internet.

EDIT: Damn. Twicker is swinging that banhammer pretty hard, lately.
Last edited on
Helios,

I'm sorry, but I feel that your interpretation is of what constitutes something that *is* vs. something that *does* is incorrect.

A mechanic is a FixEngine, or a FixWhatever. The mechanic has no purpose other than what it *does*. It does something to something else and that is all we care about. We don't care about the mechanic as a thing in any way shape or form. We only care about what it does.

There are plenty of examples in c++ of classes that *do* something only. A simple example is using a smart pointer ( in whatever chosen form ).

http://www.cplusplus.com/reference/memory/unique_ptr/

It's only purpose is to DO something, like taking the ownership of a pointer and destroying it in the the destructor, etc. . It is nothing in itself other than what it does. You never say... oh, I need a smart pointers because it *is* something. You use it because it *does* something, and yet it s a class.

Regarding...

Mechanic mech;
mech.fix_engine(vehicle);
mech.fix_engine(vehicle2);


mech may or may not be in an invalid state. If fix_engine() is designed to clean itself up when done, then immediately calling fix_engine() again could be a perfectly valid thing to do. It's very, very short sighted to discount a very powerful mechanism such as a class, just because it *may* be used incorrectly by someone. It's up to the designer of the class to determine if fix_engine() may be called once, twice or however many times.

But again, going back to the original intent of this posting, calling it like

Mechanic().fix_engine( vehicle );

would ensure that Mechanic is clean before being used.

In short, a class can DO something as it's main purpose. There are millions of examples of a class that is deigned to DO something. All of this talk about needing and interface, etc. is trivial and obvious. Of course it needs and interface and every example demonstrated in this thread has an interface/ The interface can be a single function call if need be, or it can have many functions/variables to interact with the outside world. It all depends on the problem and what is required. I feel that your interpretation of these things is well intended, but lacks deeper thought and experience.

The only true wisdom is in knowing you know nothing.

Socrates


There is no rule, and no measurement to determine when a Mechanic class ceases to be something that *does* vs. something that *is*. If using a class is the most elegant and appropriate solution to solving a problem, then it should be used.

Anyway, that's the last I'm going to say on the topic.

p.s. I have no problem with constructive criticism, but I will call out rudeness when I see it. I'll stay on the internet as much as I like than you very much.
I'm sorry, but I feel that your interpretation is of what constitutes something that *is* vs. something that *does* is incorrect.
I'm the one who gave the rule and I don't understand what I myself said?

A mechanic is a FixEngine, or a FixWhatever. The mechanic has no purpose other than what it *does*. It does something to something else and that is all we care about. We don't care about the mechanic as a thing in any way shape or form. We only care about what it does.
You don't have enough information about the problem that Mechanic models to make that judgement call. I'm the one who put forth the example, and I say that a Mechanic is an entity that has interest beyond the things it can do.

To give another example, consider
1
2
3
4
5
class Prng{
public:
	Prng(unsigned seed);
	unsigned next();
};
A Prng does something, but it also is something. If this wasn't the case, one Prng would be the same as any other. But because of the nature of PRNG this is clearly false. A PRNG seeded with 42 is obviously different from one seeded with 37. If you're doing random testing and you find a bug in the program when the PRNG for the input is seeded with a specific number, when you'll try to reproduce the bug you will very much care if the PRNG is seeded with that specific number again, and not with some other number.

There are plenty of examples in c++ of classes that *do* something only. A simple example is using a smart pointer ( in whatever chosen form ).

http://www.cplusplus.com/reference/memory/unique_ptr/

It's only purpose is to DO something, like taking the ownership of a pointer and destroying it in the the destructor, etc. . It is nothing in itself other than what it does. You never say... oh, I need a smart pointers because it *is* something. You use it because it *does* something, and yet it s a class.
That's a very bad example for your argument. A std::unique_ptr<T> does something (or, more specifically, will do something upon destruction), but it also is something in its own right. For the same T, two std::unique_ptr<T> are not interchangeable.
1
2
3
4
5
auto s1 = std::make_unique<std::string>("cp a.txt b.txt");
auto s2 = std::make_unique<std::string>("rm -rf /");
if (rand() % 2)
	std::swap(s1, s2);
system(s1->c_str());
At line 5, we really care whether the std::swap() call happened!

If fix_engine() is designed to clean itself up when done, then immediately calling fix_engine() again could be a perfectly valid thing to do. It's very, very short sighted to discount a very powerful mechanism such as a class, just because it *may* be used incorrectly by someone.
You're missing my point. What I'm getting at is that if no function of a class' interface is ever capable of changing the internal state of the class, then the class could be refactored into one or more functions without losing anything. For example, if Mechanic has only one function (fix_engine()) which always does the same thing, then extracting the function from the class doesn't lose anything. As another example,
1
2
3
4
5
6
7
class Prng{
public:
	Prng(unsigned seed){}
	unsigned next(){
		return 360391561;
	}
};
can be refactored into
1
2
3
unsigned Prgn_next(){
	return 360391561;
}
and nothing is lost.
However, I conceded that the Mechanic class in that example was questionable but probably acceptable.

But again, going back to the original intent of this posting, calling it like

Mechanic().fix_engine( vehicle );

would ensure that Mechanic is clean before being used.
And would, necessarily, be equivalent to Mechanic_fix_engine( vehicle );, except doing it in a function would not allow someone to do
1
2
3
Mechanic mech;
mech.fix_vehicle(vehicle);
mech.fix_vehicle(vehicle2);
which might not be valid.

I feel that your interpretation of these things is well intended, but lacks deeper thought and experience.
I don't recall ever making any personal attacks.

There is no rule, and no measurement to determine when a Mechanic class ceases to be something that *does* vs. something that *is*.
Sure there is. A class is something if it's stateful. If it's not stateful, it doesn't need to be a class.

I have no problem with constructive criticism, but I will call out rudeness when I see it.
I'm sorry. In the future I will try not to be rude to code. I now know sensitive software is to having its feelings hurt.
I'm sorry, but I feel that your interpretation is of what constitutes something that *is* vs. something that *does* is incorrect.

It is commonly accepted convention that objects represent objects. Classes are (should usually) be named as nouns. If they are not, the class probably represents a template metafunction or some morphism (a functor, expression template, etc.) borrowed from category theory. Since C++11 and 14, functors and metafunctions have become much less common, thanks mainly to decent closures and constexpr.

There are plenty of examples in c++ of classes that *do* something only. A simple example is using a smart pointer ( in whatever chosen form ).
unique_ptr provides a service, but it is not a service.
A unique_ptr represents a unique referent and the explicit ownership of that referent. Nobody is debating that this is the case, yet the key point is that it (as a class) represents something. It is.

This is the point helios is making. What does a "FixEngine" object represent?
Maybe you created it because fixing an engine requires complex state. If this is the case, than you've described some sort of morphism. Not quite a class (except for the fact that it maybe uses the keyword), but I could perhaps make a case for it. In C++, I would expect such an object to be returned as a pending computation from something - perhaps something like diagnose_and_defer_fix_engine() (it's a bad example name, sorry).

If the process of fixing an engine doesn't require complex state -- it's just a computation, than you're abusing the object-oriented facilities of your language. Write a function instead. If something (i.e., a mechanic) is responsible for fixing the engine, than make that function a member.
Last edited on
I agree with everything mbozzi has said. I would just like to add that functors also are, since they are stateful. Rewriting my Prng example as a functor:
1
2
3
4
5
class Prng{
public:
	Prng(unsigned seed);
	unsigned operator()();
};


Metafunctions such as those from <type_traits> technically are also an abuse of classes, because they're classes that are never meant to be instantiated as objects, but are only meant to change the state of the compiler to generate code. Different coding guidelines may disagree on whether template metaprogramming should be allowed. It's worth noting that there are things that can be done with template metaprogramming that absolutely cannot be done any other way, other than through a separate build step to generate code.
Topic archived. No new replies allowed.