patterncppMinor
Classes to manage perks to be added and removed
Viewed 0 times
perksremovedaddedmanageclassesand
Problem
I am looking for code correctness and design usage as I think I might be over doing it in the class department.
There are two things I'm mostly concerned with.
-
The possible redundancy of classes derived from
the rows/columns in its internal perk list. I'm thinking I should
just have a concrete PerksList class instead.
-
The use of an
different indices associated with the
will follow the same order, so the indices will always mean the same
thing regardless of which
PerkSlotEnums.h
IPerks.h
CommonPerks.cpp
```
#include "CommonPerks.h"
#include "PerkSlotEnums.h"
CommonPerks::CommonPerks(void)
{
perkList.resize((int)PERK_SLOTS::COMMON_SLOT_COLUMNS);
perkList[(int)PERK_SLOTS::SCOPE_SLOT_INDEX].resize((int)PERK_SLOTS::COMMON_SCOPE_PERK_COUNT);
}
CommonPerks::~CommonPerks(void)
{
}
void CommonPerks::AddPerk(IPerk* perk, int column, int row)
{
perkList[column][row] = perk;
}
void CommonPerks::RemovePerk(unsigned int index_1, unsigned int index_2)
{
if(index_1 < perkList.size())
{
if(index_2 < perkList.at(index_1).size())
{
p
There are two things I'm mostly concerned with.
-
The possible redundancy of classes derived from
IPerks. AsCommonPerks, UncommonPerks, etc, all look identical except forthe rows/columns in its internal perk list. I'm thinking I should
just have a concrete PerksList class instead.
-
The use of an
enum to keep track of constant values for thedifferent indices associated with the
perkList. All perkList'swill follow the same order, so the indices will always mean the same
thing regardless of which
IPerks derived class is being used.PerkSlotEnums.h
enum class PERK_SLOTS
{
COMMON_SLOT_COLUMNS = 1,
COMMON_SCOPE_PERK_COUNT = 2,
SCOPE_SLOT_INDEX =0,
STATS_UPGRADE_A_SLOT_INDEX = 1,
UNIQUE_PERK_A_SLOT_INDEX = 2,
STATS_UPGRADE_B_SLOT_INDEX = 3,
UNIQUE_PERK_B_SLOT_INDEX = 4,
STATS_UPGRADE_C_SLOT_INDEX = 5,
UNIQUE_PERK_C_SLOT_INDEX = 6
};IPerks.h
#pragma once
#include "IPerk.h"
#include "IComponent.h"
#include
class IPerks : public IComponent
{
public:
IPerks(void);
virtual ~IPerks(void);
virtual void AddPerk(IPerk* perk, int column, int row) = 0;
virtual void RemovePerk(unsigned int index_1, unsigned int index2) =0;
virtual IPerk* GetPerk(unsigned int perkId) const = 0;
};CommonPerks.cpp
```
#include "CommonPerks.h"
#include "PerkSlotEnums.h"
CommonPerks::CommonPerks(void)
{
perkList.resize((int)PERK_SLOTS::COMMON_SLOT_COLUMNS);
perkList[(int)PERK_SLOTS::SCOPE_SLOT_INDEX].resize((int)PERK_SLOTS::COMMON_SCOPE_PERK_COUNT);
}
CommonPerks::~CommonPerks(void)
{
}
void CommonPerks::AddPerk(IPerk* perk, int column, int row)
{
perkList[column][row] = perk;
}
void CommonPerks::RemovePerk(unsigned int index_1, unsigned int index_2)
{
if(index_1 < perkList.size())
{
if(index_2 < perkList.at(index_1).size())
{
p
Solution
It is hard to understand what this code does without context.
Strange code
I am looking at this enum and I am asking myself - why?
Is there any way to divide this enum into maybe 3 different enums? That's really strange that you use the same numbers for different elements.
C++ is not C
Next, you have:
Remove
Conventions
Your methods starts with capital letters, that mostly not popular among C++ developers.
I suspect that
Bad naming
Your parameters are badly named, give some more meaningful names.
Pragma
Pragma is compiler dependent. I would suggest to use headers guards.
Strange code
I am looking at this enum and I am asking myself - why?
Is there any way to divide this enum into maybe 3 different enums? That's really strange that you use the same numbers for different elements.
enum class PERK_SLOTS
{
COMMON_SLOT_COLUMNS = 1,
COMMON_SCOPE_PERK_COUNT = 2,
SCOPE_SLOT_INDEX =0,
STATS_UPGRADE_A_SLOT_INDEX = 1,
UNIQUE_PERK_A_SLOT_INDEX = 2,
STATS_UPGRADE_B_SLOT_INDEX = 3,
UNIQUE_PERK_B_SLOT_INDEX = 4,
STATS_UPGRADE_C_SLOT_INDEX = 5,
UNIQUE_PERK_C_SLOT_INDEX = 6
};C++ is not C
Next, you have:
CommonPerks::CommonPerks(void)Remove
void, it is totally unneccesary. It should be simplyCommonPerks::CommonPerks()Conventions
Your methods starts with capital letters, that mostly not popular among C++ developers.
virtual void AddPerk(IPerk* perk, int column, int row) = 0;I suspect that
IComponent has a virtual destructor. You do not need to do this.virtual ~IPerks(void);Bad naming
Your parameters are badly named, give some more meaningful names.
void CommonPerks::RemovePerk(unsigned int index_1, unsigned int index_2)Pragma
Pragma is compiler dependent. I would suggest to use headers guards.
Code Snippets
enum class PERK_SLOTS
{
COMMON_SLOT_COLUMNS = 1,
COMMON_SCOPE_PERK_COUNT = 2,
SCOPE_SLOT_INDEX =0,
STATS_UPGRADE_A_SLOT_INDEX = 1,
UNIQUE_PERK_A_SLOT_INDEX = 2,
STATS_UPGRADE_B_SLOT_INDEX = 3,
UNIQUE_PERK_B_SLOT_INDEX = 4,
STATS_UPGRADE_C_SLOT_INDEX = 5,
UNIQUE_PERK_C_SLOT_INDEX = 6
};CommonPerks::CommonPerks(void)CommonPerks::CommonPerks()virtual void AddPerk(IPerk* perk, int column, int row) = 0;virtual ~IPerks(void);Context
StackExchange Code Review Q#77765, answer score: 2
Revisions (0)
No revisions yet.