patterncppMinor
Acceptable use of typedef in class?
Viewed 0 times
useacceptabletypedefclass
Problem
Is this an acceptable use of typedef in the class, Inventory?
Inventory.h
Inventory.cpp
Inventory.h
#ifndef INVENTORY_H
#define INVENTORY_H
#include "Item.h"
#include
class Inventory
{
public:
typedef std::unordered_map ItemTable; ///< Items and their quantities
const ItemTable& items() const;
void add_item(Item item, int quantity);
int quantity(Item item);
bool remove_item(Item item, int quantity);
private:
ItemTable items_;
};
#endifInventory.cpp
#include "Inventory.h"
#include "Item.h"
#include
const Inventory::ItemTable& Inventory::items() const
{
return items_;
}
void Inventory::add_item(Item item, int quantity)
{
int total_of_item = items_[item];
items_[item] = total_of_item + quantity;
}
int Inventory::quantity(Item item)
{
return items_[item];
}
bool Inventory::remove_item(Item item, int quantity)
{
int total_of_item = items_[item];
if (quantity > total_of_item) {
return false;
}
items_[item] = total_of_item - quantity;
return true;
}Solution
Yes. I like it because it will make maintenance easier.
If you change the type of the container then you only need to change it in one place in the code. I would even go and define other things in relation to the typedef.
But I would not expose the internal object with this:
Any object that uses this method not becomes very tightly bound to this interface. I would question the need for this functionality. I prefer to expose methods that manipulate the object as a whole.
An alternative is to expose the iterators. Iterators are a much looser coupling.
As a result I would make
Edit:
If you change the type of the container then you only need to change it in one place in the code. I would even go and define other things in relation to the typedef.
// For example:
typedef ItemTable::iterator Iter;
typedef ItemTable::const_iterator CIter;But I would not expose the internal object with this:
const Inventory::ItemTable& Inventory::items() constAny object that uses this method not becomes very tightly bound to this interface. I would question the need for this functionality. I prefer to expose methods that manipulate the object as a whole.
An alternative is to expose the iterators. Iterators are a much looser coupling.
As a result I would make
ItemTable a private type. While the Iter and CIter would be public.Edit:
class X
{
typedef std::vector Cont;
Cont data;
public:
typedef Cont::iterator Iter;
typedef Cont::const_iterator CIter;
Iter begin() { return data.begin();}
Iter end() { return data.end();}
CIter begin() const { return data.begin();}
CIter end() const { return data.end();}
};Code Snippets
// For example:
typedef ItemTable::iterator Iter;
typedef ItemTable::const_iterator CIter;const Inventory::ItemTable& Inventory::items() constclass X
{
typedef std::vector<int> Cont;
Cont data;
public:
typedef Cont::iterator Iter;
typedef Cont::const_iterator CIter;
Iter begin() { return data.begin();}
Iter end() { return data.end();}
CIter begin() const { return data.begin();}
CIter end() const { return data.end();}
};Context
StackExchange Code Review Q#13138, answer score: 7
Revisions (0)
No revisions yet.