Otdfbt

I have an image class and a table class. To each image a single table can be "attached". Each <image, table> pair should be identified with an ID, which can later be used to get the pointer of image or table associated with that ID. Images in this "map" should be unique. Below is my solution, please let me know if it can be improved. Thanks.

browserInfo.h

#include <vector>
#include <tuple>

class BrowserInfo

public:
 // Returns a unique ID for the <image, table> pair
 /*
 @param img The image file pointer
 @param tbl The table view pointer
 @return A unique ID for the input pair
 */
 unsigned getId(image *img, table *tbl) const;

 /// Returns a image pointer for the given ID
 /*
 @param uId The ID of <image, table> pair
 @return A pointer to image file if ID exists, otherwise nullptr
 */
 image *getImage(unsigned uId) const;

 /// Returns a table pointer for the given ID
 /*
 @param uId The ID of <image, table> pair
 @return A pointer to table if ID exists, otherwise nullptr
 */
 table *getTable(unsigned uId) const;

private:
 // alias for the type
 using imageTableToId = std::tuple<image *, table *, unsigned>;

 // This vector keeps track of all unique <image, table> pair IDs
 mutable std::vector<imageTableToId> m_vecImageTableIds;

 // The current ID
 mutable unsigned m_iCurrentId = 0;

; // class BrowserInfo

browserInfo.cpp

unsigned BrowserInfo::getId(image *img, table *tbl) const

 // first, try to see if we have worked with the provided image before
 for (auto &tuple : m_vecImageTableIds)
 
 if (std::get<0>(tuple) == img)
 
 // we support a single table view for each image.
 // therefore, if we find that the image is already stored
 // in our vector, we just need to update the corresponding
 // table pointer and return a new unique ID for this pair
 std::get<1>(tuple) = tbl;
 std::get<2>(tuple) = ++m_iCurrentId;

 return m_iCurrentId;
 
 

 // if we got here it means the image pointer wasn't stored before
 // so we can just insert a new tuple into the vector
 m_vecImageTableIds.push_back(std::make_tuple(img, tbl, ++m_iCurrentId));
 return m_iCurrentId;


image *BrowserInfo::getImage(const unsigned uId) const

 for (const auto &tuple : m_vecImageTableIds)
 
 if (std::get<2>(tuple) == uId)
 return std::get<0>(tuple);
 

 return nullptr;


table *BrowserInfo::getTable(const unsigned uId) const

 for (const auto &tuple : m_vecImageTableIds)
 
 if (std::get<2>(tuple) == uId)
 return std::get<1>(tuple);
 

 return nullptr;

While your solution works, it can be made even more understandable.

Instead of using a vector of tuples, consider using an std::map:

std::map<unsigned, std::tuple<image *, table *>> imageTablePairs;

This is the most natural expression of the problem statement: mapping an ID to an image-table pair. It will also simplify the logic of your getId function. Here's an algorithm for what you need to do:

Cycle through the map and check if the image exists. If it does, delete its record from the map (using std::map::erase).

Then, simply do:

imageTablePairs[++m_iCurrentId] = std::make_tuple(img, tbl);

This covers both the case when the image exists (in which case its old record gets deleted per the algorithm above) and the case when the image does not exist (in which case we simply make a new record).

For the other two getter functions, we obviously can't assume that the ID being passed in exists in the map, so we can create a private helper function that takes an ID and returns true if it exists and false otherwise. Then, the logic becomes:

• getImage: if the ID exists, return std::get<0>(imageTablePairs[uId]).




• getTable: if the ID exists, return std::get<1>(imageTablePairs[uId]).

If I misunderstood the problem statement and this solution is not possible, please let me know.

Edit: Here's the code I'd use. Tested in Visual Studio 2017 and confirmed that it compiles and runs as expected (I used empty image and table structs for testing).

browserInfo.h

#pragma once
#include <map>
#include <tuple>

class BrowserInfo

public:
 // Returns a unique ID for the <image, table> pair
 /*
 @param img The image file pointer
 @param tbl The table view pointer
 @return A unique ID for the input pair
 */
 unsigned getId(image *img, table *tbl) const;

 /// Returns a image pointer for the given ID
 /*
 @param uId The ID of <image, table> pair
 @return A pointer to image file if ID exists, otherwise nullptr
 */
 image *getImage(unsigned uId) const;

 /// Returns a table pointer for the given ID
 /*
 @param uId The ID of <image, table> pair
 @return A pointer to table if ID exists, otherwise nullptr
 */
 table *getTable(unsigned uId) const;

private:

 bool idExists(unsigned uId) const;

 mutable std::map<unsigned, std::tuple<image *, table *>> imageTablePairs;

 // The current ID
 mutable unsigned m_iCurrentId = 0;

; // class BrowserInfo

browserInfo.cpp

#include "browserInfo.h"


unsigned BrowserInfo::getId(image * img, table * tbl) const

 for (auto &record : imageTablePairs)
 
 if (std::get<0>(record.second) == img)
 
 imageTablePairs.erase(record.first);
 break;
 
 

 imageTablePairs[++m_iCurrentId] = std::make_tuple(img, tbl);
 return m_iCurrentId;


image * BrowserInfo::getImage(unsigned uId) const

 if (idExists(uId))
 
 return std::get<0>(imageTablePairs[uId]);
 

 return nullptr;


table * BrowserInfo::getTable(unsigned uId) const

 if (idExists(uId))
 
 return std::get<1>(imageTablePairs[uId]);
 

 return nullptr;


bool BrowserInfo::idExists(unsigned uId) const

 std::map<unsigned, std::tuple<image*, table*>>::iterator it = imageTablePairs.find(uId);
 return it != imageTablePairs.end();

As every id identifies a tuple of unique image, optional table, why over-complicate things?

Select one alternative from here:

1. Add a table* to image.


2. Use a std::map or std::unordered_map from image* to table*.

And one from here:

1. Make the ids image*s.


2. Add a (potentially optional) id to image. Just ensure that you can search them by id.


3. Use a map from arbitrary id to image* (and optionally in reverse).

Presto, you are done, and this answer will be far longer and more complicated than the solution.

As an alternative, if you want multiple indices into the same collection, look at Boost.MultiIndex.