xatja
xatja

Reputation: 23

Should a typedef be both in the class definition and class declaration?

I'm learning C++. I know that C++ is a bit more verbose than most other languages, however defining a typedef in a struct/ class declaration and then again in the struct/ class definition got me thinking if there isn't a better or more central place for the typedef where it only has to be defined once, and whether this follows what is thought of as C++ best practices. I couldn't find anything in particular about this on the web.

Moving the typedef to a more global place seems inappropriate and the typedef is both used for the definition and implementation of the method within the struct.

//--ProductFilter.h--
struct ProductFilter {
    typedef std::vector<Product*> Items;
    static Items by_color(Items items, Color color);
};

//--ProductFilter.cpp--
typedef std::vector<Product*> Items;
Items ProductFilter::by_color(Items items, Color color) {
    Items result;
    for (auto& item : items)
        if (item->color == color)
            result.push_back(item);
    return result;
}

Upvotes: 2

Views: 793

Answers (5)

Arne J
Arne J

Reputation: 415

There is nothing wrong with wanting to get the type out of that class in the cpp for practical reasons. You just should not redefine, but alias it instead :

// top of cpp file
using Items = ProductFilter::Items;

If the type of Items ever changes, the alias will not need to be rewritten.

Upvotes: 0

Jarod42
Jarod42

Reputation: 217398

Your definition should be:

ProductFilter::Items ProductFilter::by_color(Items items, Color color)
{
    // ...
}

or, with trailing return type:

auto ProductFilter::by_color(Items items, Color color) -> Items 
{
    // ...
}

Upvotes: 2

NathanOliver
NathanOliver

Reputation: 180640

You don't need to defined the typedef in the class declaration and class definition. The reason you are doing so is that when you do

Items ProductFilter::by_color(Items items, Color color) {
    Items result;
    for (auto& item : items)
        if (item->color == color)
            result.push_back(item);
    return result;
}

The Items for the return type is not scoped to the class, so Items needs to be a name in the global scope. If you use

ProductFilter::Items ProductFilter::by_color(Items items, Color color) {
    Items result;
    for (auto& item : items)
        if (item->color == color)
            result.push_back(item);
    return result;
}

Then you no longer have this issue and you can just have the typedef in the class declaration.


The reason you don't need to do ProductFilter::Items in

ProductFilter::by_color(Items items, Color color)

is because the ProductFilter:: puts you in the scope of the class so you can use names defined in the class without having to qualify them.

Upvotes: 2

Vlad from Moscow
Vlad from Moscow

Reputation: 310980

Just use the qualified name of the typedef outside the class scope

ProductFilter::Items ProductFilter::by_color(Items items, Color color) {
    Items result;
    for (auto& item : items)
        if (item->color == color)
            result.push_back(item);
    return result;
}

Upvotes: 1

463035818_is_not_an_ai
463035818_is_not_an_ai

Reputation: 122516

Your two typdefs actually declare two different aliases for the same type. One is ProductFilter::Items the other is Items in the global namespace.

Actually I am not sure of the rules which one you get when writing Items within the scope of ProductFilter, though it doesnt really matter as they are identical anyhow.

You only need either of them. Which one you pick depends on in which scope you want declare it. If Items semantically belongs to ProductFilter then I'd choose the first and put it in the declaration only.

You would have to qualify the return type as ProductFilter::Items (or use trailing return type).

Upvotes: 1

Related Questions