public class Chest
{
public List<Thing> things;
public void ModifyThings() {}
}
About this ModifyThings() API, which signature do you think is better/easier to use? (Suppose that we want to paint yellow each thing in the chest that is a box)
A predicate to decide which thing will the action run
Both are clunky foreach loops. Both has chests work differently than other containers, for dubious reasons. Both allocate a lambda every time you call them, so if you care about performance over ease of use, they’re bad. Neither interact with Linq, so if you care about ease of use over performance, they’re bad.
Stone age code that’s better than both alternatives:
public void ChangeBoxColor()
{
foreach (var thing in chest.things)
if (thing.isBox)
thing.color = Color.yellow;
}
It’s a lot less code, and you don’t need to know a special API for chest in order to do the same thing that you do with everything else.
If the concern is that you don’t want outside things to modify chest.things, and just allow modifications of the things inside, do:
public class Chest
{
private List<Thing> things;
public IReadOnlyList<Thing> Things => things;
}
If you really want to do the thing for some reason, then the thing that gives you all the objects instead of the filter is more flexible, but it doesn’t really matter that much?
I’ve tried to make a simple example but I think that I have made things worst hehe. Let me try again, the point isn’t exactly how to operate over the container, the actual case is that I have modifiers being hooked into Things using a dictionary of predicates and actions, something like this
public static Dictionary<Func<Drop, bool>, List<Action<Drop>>> globalModifiers =
new Dictionary<Func<Drop, bool>, List<Action<Drop>>>();
And this is being used in a custom enumerable class (Chest), something like this:
public static IEnumerable<Drop> ApplyGlobalModifiers (this IEnumerable<Drop> drops)
{
foreach (var drop in drops)
{
foreach (var entry in DropTable.globalModifiers)
{
if (!entry.Key.Invoke(drop))
continue;
foreach (var action in entry.Value)
{
action.Invoke(drop);
}
}
yield return drop;
}
}
This is part of a plugin, so user will do something like:
table.AddGlobalModifier(drop => drop.Entry is Currency,
drop => drop.AmountVariation *= 2);
But then I thought that maybe was a better decision to let user add modifiers with only one parameter, and then manually decide from which item they will do something or not. So, which case in your opinion is better?
The second case (when we have only one parameter) get better, in my opinion, cause its less thing to document and less thing that user need to remember, so it reduces the change to they do something wrong, but by another side, the first option is better to allow the designer to tinker things inspector level, because in second a programmer is necessary, but in first I can drag and drop an predicate to when do something and drag and drop what
Okay, that makes more sense. So our alternatives are:
table.AddGlobalModifier(drop => drop.Entry is Currency,
drop => drop.AmountVariation *= 2);
And
table.AddGlobalModifier(drop => {
if (drop.Entry is Currency)
drop.AmountVariation *= 2
});
The second one is for sure more flexible. It’s also totally possible to build exactly the same drag-and-drop inspector thing you’re talking about by just creating a wrapper that takes a type and a modifier:
AddModifierForType(DropTable table, Type type, Action<Drop> modifier) {
table.AddGlobalModifier(drop => {
if (drop.Entry.GetType() == type)
mo(drop);
});
}
That being said, this seems a bit crazy. I assume Drop is a class, otherwise your code just doesn’t do anything. But in that case you can only run ApplyGlobalModifiers once, as the operation is destructive. What if the modifiers change? Or is that just not a thing that can happen at runtime?
Also, does the same lambda as a key in two different instances give the same hash? Ie. if you use drop => drop.Entry [is](http://www.google.com/search?q=is+msdn.microsoft.com) Currency in two places to grab the list from the dictionary, do you get the same list?
And finally, use Predicate instead of Func<T, bool>, as it makes things surprisingly much easier to read.
You’re totally right, I’ll stick with the second one
Not exactly, this is being run when iterating over DropTable in a clone of drops’ container, something like this (actually this isn’t the real code, I’m changing this for some limitations problems, but the idea is similar)
public IEnumerator<Drop> GetEnumerator()
{
// Create a copy of drops
var dropsCopy = drops
.Select(d => d.Clone())
.Cast<Drop>()
.ToList();
// Apply things on drops
var customDrops = dropsCopy
.ApplyGlobalModifiers()
.ApplyLocalModifiers(this)
.FilterByRules(this)
.ToList();
var modifiedAndFilteredDrops = new ModifyEventArgs() {drops = customDrops};
RaiseModifiedAndFiltered(modifiedAndFilteredDrops);
return customDrops.GetEnumerator();
}
I don’t get this point, modifiers are expected to change during runtime, they can be added and removed by API’s
Nps, if you insert lambdas as key they can’t be retrieved but will be executed, the only way to remove they are clearing the dictionary, in the documentation I’m suggesting that user set modifiers as named methods to enable us to add/remove they
You’re right, initially, I was using a predicate, but as I have some things that do not receive any parameter and return a bool and I can’t simply have a Predicate without any T, I finished with something like
public Dictionary<Predicate<Drop>, List<Action<Drop>>> localModifiers =
new Dictionary<Predicate<Drop>, List<Action<Drop>>>();
public Dictionary<Func<bool>, List<Func<Drop, bool>>> rules =
new Dictionary<Func<bool>, List<Func<Drop, bool>>>();
This sounds like the first parameter is different in some way, so I changed it to Func in all places instead of mix them