0

I have a class with about 10 or more different boolean values that record whether a user has done a particular action that will give them a specific reward (e.g. send a message to someone).

here is the code for one method for ONE bool/action:

private ReqRewardResult setMsgSent(RewardClass reward, RewardInfo info)
{
    if (reward.msgSent)
        return ReqRewardResult.RewardAlreadyGiven;

    reward.msgSent = true;
    reward.earned += info.msgSentReward;
    return ReqRewardResult.ReqSuccess;
}

I have tried to create a generic method for this but it seems you can't pass a class variable as a reference?

private ReqRewardResult setRewardAction(ref bool bAction, RewardClass reward, int reward)
{
    if (bAction)
        return ReqRewardResult.RewardAlreadyGiven;

    bAction = true;
    reward.earnedTokens += reward;
    return ReqRewardResult.ReqSuccess;
}

I have then looked at a couple of methods such as using a delegate function... but this is then kinda pointless as i'd have to repeat several lines again...

I have also seen you could use Reflection... but this is really slow and as this is a web app i'd rather use more repeated code if it improves the overall speed...

The question: Is there anyway to have a class function that can repeat for several variables of the same type without any performance hit?

NOTE: This class is data that is loaded from a database and is unique to each user (there could be millions of users)

Many Thanks,

Phil.

philkills
  • 47
  • 7
  • 1
    You have an error in you code example, the parameter with name reward is duplicate (second one example) – Che Jul 27 '18 at 13:23
  • How is this related to asp net? – pollirrata Jul 27 '18 at 14:26
  • I honestly am not sure what problem you're trying to solve here. Can you explain better what you're trying to accomplish? C# doesn't have "functions" it has "methods". "repeat for several variables of the same type" -- this is what methods do. I'm not sure what you're conveying here. Without performance hit? No, everything will add some performance hit – Joe Phillips Jul 27 '18 at 14:31
  • @JoePhillips right now I have the same code for 10 different booleans that represent different actions a user on the web may take... in the example above the boolean is called msgSent.... but could be called anything such as phonedFriend LikedPost CommentedOnPost etc.... Each time the user performs 1 of these actions they get rewarded based on the specific action they performed (but can only be rewarded ONCE... hence the boolean) – philkills Jul 27 '18 at 14:45
  • @philkills So what's the problem? In the DB you should have a nullable field. If it's null, that means no action has ever been taken, if its false or true, act accordingly. Write code that checks and does those things – Joe Phillips Jul 27 '18 at 14:49
  • How are you using this? Could you show some of the original methods *including* how you call them, just as an example? – Hans Kesting Jul 27 '18 at 15:24
  • @JoePhillips The problem is NOT how to solve this... its how to solve it in a CODE efficient way without compromising on PERFORMANCE.... Right now its fast but have repeated the same code but for different variables about 10 times.... – philkills Jul 27 '18 at 15:59

2 Answers2

0
class RewardCredit
{
   public bool Rewarded { get; set; }

   public int Points { get; set; }
}

You could use a Dictionary<string, RewardCredit>, fill it with reward names as strings and have a function like

void ApplyReward(string rewardName)
{
    if (!rewards.ContainsKey(rewardName))
    {
        return;
    }

    RewardCredit credit = rewards[rewardName];
    if (!credit.Rewarded)
    {
        tokens += credit.Points;
        credit.Rewarded = true;
    }
}

You'd then end up using it like so

Dictionary<string, RewardCredit> rewards = new Dictionary<string, RewardCredit>
{
    { "Message", new RewardCredit { Points = 10 } },

};

ApplyReward("Message");
Biesi
  • 800
  • 9
  • 17
  • Now that I think about it further, you could even use an array and access the reward info by using an enum value as the index – Biesi Jul 27 '18 at 14:06
  • I forgot to mention this is actually a database table also :/ But good answer otherwise. – philkills Jul 27 '18 at 14:19
  • @philkills so what's the problem? You just need to handle the reading and writing logic differently then – Biesi Jul 27 '18 at 14:22
  • The problem with that would be performance and scalability of the app... E.g. 2 ways of doing your implementation: 1. Create a new table that holds 1 boolean and for each user you have a list of booleans... = slower read/write to the database. 2. When reading from the database convert to a dictionary the whole table or booleans... again as each operation of "add reward" only happens for 1 boolean per request from the client this is still slower... – philkills Jul 27 '18 at 14:39
  • It sounds to me you're looking for a single field that can update multiple values, like this: https://stackoverflow.com/questions/8447/what-does-the-flags-enum-attribute-mean-in-c I don't necessarily agree that is the best answer but that's what you seem to be asking for – Joe Phillips Jul 27 '18 at 14:51
  • @philkills if you only want to read one value with each call you can still tweak this solution to work on your database table instead of the `Dictionary`. You just need to implement the logic for reading the column based on the reward. You could do this by passing the column names to the `ApplyReward` function or have some mapping. – Biesi Jul 28 '18 at 11:28
0

First it looks like you have a finish and known set of possible user actions that can be rewarded with "tokens" if done by a user. This can be implemented with the following class:

public sealed class UserAction
{
    public static UserAction SendMessage = new UserAction(3);
    public static UserAction PhonedFriend = new UserAction(4);
    public static UserAction LikedPost = new UserAction(1);
    public static UserAction CommentedOnPost = new UserAction(2);
    private UserAction(int tokens) => Tokens = tokens;
    public int Tokens { get; }
}

Notes:

  • The constructor is made private and only the few instances of this class that must exist are available with predefined names and tokens
  • The class is sealed to avoid being inherited, so that it effectively is impossible to create other instances than the one you provided

Then, so far I understand, each user may have done each action (or not) but must be rewarded no more than once for each. This make me think that you need to store a set of these UserActions into a User. You also need a way to add an action to a user (when he effectively does in on the UI, for example) and also be able to know how much tokens each user has earned. The following class implements such behavior:

public sealed class User
{
    private readonly HashSet<UserAction> _doneActions;
    public User() => _doneActions = new HashSet<UserAction>();
    public ReqRewardResult AddAction(UserAction action) => _doneActions.Add(action) ? ReqRewardResult.ReqSuccess : ReqRewardResult.RewardAlreadyGiven;
    public int EarnedTokens => _doneActions.Sum(ua => ua.Tokens);
}

Note:

  • I'm taking advantage of the HashSet.Add()'s return value to determine if the action had already been done in the past or not

Advantages of this "object-thinky" solution:

  • No more booleans and if, which I believe makes your code more maintainable and clear
  • Simplicity to add a new UserAction (one LoC)
Spotted
  • 4,021
  • 17
  • 33
  • This looks good, How would you store this is a database though with maintaining efficient read/write capability... especially as for each 'action' that is recorded it will need to READ from the db and then WRITE back to the database as they are requested using an API to a server. – philkills Jul 27 '18 at 16:08
  • @philkills answer to this question is highly dependant on which tool you use for persistence. It may wary to small tweaks in `UserAction` to writing a completely new and different persistence model. – Spotted Jul 27 '18 at 19:33