Problem
Based on feedback on my previous question, I made many small adjustments to the code, added more APIs and tried to follow through with delivering on the excellent advice I have received in regards to my code. I have updated my code with small summaries to provide more documentation.
Here is the updated interface.
namespace Constrainable
{
public interface IConstrainableSet<T>
{
T AlterInput(T Value);
T AlterInputConstrainedStore(T Value);
ConstrainableSet<T> ChainLinkClone();
ConstrainableSet<N> ChainLinkNew<N>(Predicate<N> CheckBeforeSet = null, Func<N, N> ModifyBeforeSet = null, Action<ConstrainableSet<N>> FailAction = null, Action<ConstrainableSet<N>> SuccessAction = null);
T ConstrainedStore(T Value);
T GetStoredData();
bool IsValid(T Value);
}
public interface IConnectable
{
public object Root { get; set; }
public object Next { get; set; }
public object Parent { get; set; }
}
}
And here is the new code.
using System;
namespace Constrainable
{
public class Connectable : IConnectable
{
public object Root { get; set; }
public object Next { get; set; }
public object Parent { get; set; }
}
/// <summary>
/// ConstrainableSet is intended to Allow full control over what data gets set in the inner field `data,
/// You can optioning modify the input. This is also intended to be a test of idea's,
/// new and/or interesting ways of accomplishing tasks.
/// </summary>
/// <typeparam name="T"></typeparam>
public class ConstrainableSet<T> : Connectable, IConstrainableSet<T>
{
public ConstrainableSet(Predicate<T> CheckBeforeSet = default, Func<T, T> ModifyBeforeSet = default, Action<ConstrainableSet<T>> FailAction = default, Action<ConstrainableSet<T>> SuccessAction = default)
{
_failAction = FailAction;
_checkBeforeSet = CheckBeforeSet;
_modifyBeforeSet = ModifyBeforeSet;
_successAction = SuccessAction;
}
#region ControlFlow
/// <summary>
///`_failAction` field recieves the `FailAction` parameter in the constructor.
///This Parameter is a callback that is run when a failure occurs in `ConstrainedStore()`,
///for both Callbacks the whole `ConstrainableSet` Object is passed as a parameter.
/// </summary>
private readonly Action<ConstrainableSet<T>> _failAction;
/// <summary>
/// _successAction field recieves the `SuccessAction` parameter in the constructor.
/// This Parameter is a `Callback` , this function is called on in `ConstrainedStore()`
/// and only after successful set of the inner private field `data`.
/// </summary>
private readonly Action<ConstrainableSet<T>> _successAction;
/// <summary>
/// _checkBeforeSet is a bool lambda expression which evaluates the Parameter
/// to be set, and must return true for the data to be set
/// </summary>
private readonly Predicate<T> _checkBeforeSet;
/// <summary>
/// _modifyBeforeSet this routine Takes type T as input and returns type T
/// this function is for Encryption or Concatenating or anything that involves modifying the parameter passed in to it
/// </summary>
private readonly Func<T, T> _modifyBeforeSet;
#endregion
private T data;
/// <summary>
/// The ConstrainableStore Function is the combined input of
/// 3 parameters in this fashiion (PreCheck ? SuccessAction : FailAction)
/// if there is no _checkBeforeSet function defined then this simply returns the value passed in,
/// if there is an _checkBeforeSet function defined, it calls that function and based on success or failure.
/// if that Action is defined for success it moves the value into storage then calls SuccessAction,
/// if there is no success action it sets the successful checked value and returns the value,
/// if there is no FailAction defined at this point then it returns the default value for this type
/// </summary>
/// <param name="Value"></param>
/// <returns>T</returns>
public virtual T ConstrainedStore(T Value)
{
if (_checkBeforeSet != default)
{
if (_checkBeforeSet(Value))
{
if (_successAction != default)
{
data = Value;
_successAction(this);
}
data = Value;
return Value;
}
if (_failAction != default)
{
_failAction(this);
}
return default;
}
return Value;
}
/// <summary>
/// IsValid runs the define checkBeforeSet function and returns result.
/// </summary>
/// <param name="Value"></param>
/// <returns>bool</returns>
public virtual bool IsValid(T Value)
{
if (_checkBeforeSet != null)
{
return _checkBeforeSet(Value);
}
return false;
}
/// <summary>
/// This is the interesting function,
/// I think with this simple function actionable values can be achieved
/// many tasks could benefit from Actionable `Validated` values imho.
/// Actionable values is values that can be used in some way(s) to get a new value or object,
/// or encrypting the value passed in, this could be used for something like that.
/// </summary>
/// <param name="Value"></param>
/// <returns></returns>
public virtual T AlterInput(T Value)
{
if (_modifyBeforeSet != default)
{
return _modifyBeforeSet(Value);
}
return Value;
}
//Combined usage
public virtual T AlterInputConstrainedStore(T Value)
{
return ConstrainedStore(AlterInput(Value));
}
/// <summary>
/// I made a simple way to link objects together
/// I simply needed to pinpoint Cloning and Creation and
/// push the proper values to these objects during cloning or creation.
/// </summary>
/// <returns></returns>
public ConstrainableSet<T> ChainLinkClone()
{
if(Root == default)
{
Root = this;
}
ConstrainableSet<T> newpo = new ConstrainableSet<T>(_checkBeforeSet, _modifyBeforeSet, _failAction, _successAction);
IConnectable Iterator;
if (Next != default)
{
Iterator = Next as IConnectable;
while (Iterator.Next != default)
{
Iterator = Iterator.Next as IConnectable;
}
Iterator.Next = newpo;
newpo.Root = Root;
newpo.Parent = this;
return newpo;
}
else
{
Next = newpo;
newpo.Root = Root;
newpo.Parent = this;
return newpo;
}
}
public ConstrainableSet<N> ChainLinkNew<N>(Predicate<N> CheckBeforeSet = default, Func<N, N> ModifyBeforeSet = default, Action<ConstrainableSet<N>> FailAction = default, Action<ConstrainableSet<N>> SuccessAction = default)
{
if (Root == default)
{
Root = this;
}
ConstrainableSet<N> newpo = new ConstrainableSet<N>(CheckBeforeSet, ModifyBeforeSet, FailAction, SuccessAction);
IConnectable Iterator;
if (Next != default)
{
Iterator = Next as IConnectable;
while (Iterator.Next != default)
{
Iterator = Iterator.Next as IConnectable;
}
Iterator.Next = newpo;
newpo.Root = Root;
newpo.Parent = this;
return newpo;
}
else
{
Next = newpo;
newpo.Parent = this;
newpo.Root = Root;
return newpo;
}
}
//returns the data field.
public T GetStoredData()
{
return data;
}
}
}
I would also like to provide a example of usage. This example demonstrates the new ChainLinkNew Function which can be used in the SuccessCallback function to chain validations and data stores together, or in the way I accomplish this below. Any input on my approach is highly appreciated.
public Test()
{
ConstrainableSet<int> programmable = new ConstrainableSet<int>(
CheckBeforeSet: x => x == 100
).ChainLinkNew<int>(
CheckBeforeSet: x => x == 10 | x == 1,
ModifyBeforeSet: x => x / 10,
FailureCallBack,
SuccessCallBack)
.Root as ConstrainableSet<int>;
void FailureCallBack(ConstrainableSet<int> set)
{
}
void SuccessCallBack(ConstrainableSet<int> set)
{
}
((ConstrainableSet<int>)programmable.Next).AlterInputConstrainedStore(programmable.ConstrainedStore(100));
}
Solution
Like Pieter Witvoet I don’t quite understand which problem this code is trying to solve. There is an ObservableCollection Class that partly does the same. Together with validation logic in properties and possibly the implementation of INotifyPropertyChanged, this comes close to your solution.
Apart from this, your code has problems and can be improved.
-
It does not compile, because interface members in
IConnectableare declared as public. Interface members are always public (C# 8 will change this, but we are not quite there yet). You should always post compilable code on Code Review. -
ChainLinkNew<N>allows to add elements of another type to the collection. The purpose of generics is to provide type safety while allowing you to create type variants at design time. With this construct you are losing type safety, as the type of added elements in the set will be determined at run time. Use the class’ type parameterTinstead. If you need to add items of different types, create aConstrainableSet<object>or use a base type common to all elements. Like this, at least your constrainable set is consistent and type safe, even if the data is not. Otherwise none of them will be. -
The interface
IConstrainableSet<T>depends on its own implementation! As explained here,The point of the interfaces is to have something that is common to all implementations. By trying to do this you destroy the whole reason why interfaces exist.
Change the interface to (and adapt the implementation)
public interface IConstrainableSet<T> { ... IConstrainableSet<T> ChainLinkClone(); IConstrainableSet<T> ChainLinkNew(Predicate<T> CheckBeforeSet = null, Func<T, T> ModifyBeforeSet = null, Action<IConstrainableSet<T>> FailAction = null, Action<IConstrainableSet<T>> SuccessAction = null); ... } -
The
IConnectableinterface should be generic.public interface IConnectable<T> { T Root { get; set; } T Next { get; set; } T Parent { get; set; } } -
The
Connectableclass can beabstractand must implementIConnectable<T>.Connectableby itself does not contain data and does not seem to be useful other than as base class. If it contained aDatafield it would make sense to instantiate aConnectable. As it is now, you could only create a doubly linked list containing empty nodes.public abstract class Connectable<T> : IConnectable<T> -
These improvements require some changes in the implementation but also allow some simplifications. E.g. in
ChainLinkClonevar newpo = new ConstrainableSet<T>(_checkBeforeSet, _modifyBeforeSet, _failAction, _successAction); if (Next != default) { ConstrainableSet<T> Iterator = Next; while (Iterator.Next != default) { Iterator = Iterator.Next; } ... } else { ... }- Use
varin thenewstatement to avoid rewriting the lengthy type name. - Move
Iteratorinsideifand use an initializer. - We can drop some casts.
- Use