1

A brainteaser for you!

I am developing a modular system, in such a way that module A could need module B and module B could also need module A. But if module B is disabled, it will simply not execute that code and do nothing / return null.

A little bit more into perspective:

Let's say InvoiceBusinessLogic is within module "Core". We also have a "Ecommerce" module which has a OrderBusinessLogic. The InvoiceBusinessLogic could then look like this:

public class InvoiceBusinessLogic : IInvoiceBusinessLogic
{
    private readonly IOrderBusinessLogic _orderBusinessLogic;

    public InvoiceBusinessLogic(IOrderBusinessLogic orderBusinessLogic)
    {
        _orderBusinessLogic = orderBusinessLogic;
    }

    public void UpdateInvoicePaymentStatus(InvoiceModel invoice)
    {
        _orderBusinessLogic.UpdateOrderStatus(invoice.OrderId);
    }
}

So what I want is: When the module "Ecommerce" is enabled, it would actually do something at the OrderBusinessLogic. When not, it would simply not do anything. In this example it returns nothing so it can simply do nothing, in other examples where something would be returned, it would return null.

Notes:

  • As you can probably tell, I am using Dependency Injection, it is a ASP.NET Core application so the IServiceCollection takes care of defining the implementations.
  • Simply not defining the implementation for IOrderBusinessLogic will cause a runtime issue, logically.
  • From a lot of research done, I do not want to make calls to the container within my domain / logic of the app. Don't call the DI Container, it'll call you
  • These kind of interactions between modules are kept to a minimum, preferably done within the controller, but sometimes you cannot get around it (and also in the controller I would then need a way to inject them and use them or not).

So there are 3 options that I figured out so far:

  1. I never make calls from module "Core" to module "Ecommerce", in theory this sounds the best way, but in practice it's more complicated for advanced scenarios. Not an option
  2. I could create a lot of fake implementations, depending on the configuration decide on which one to implement. But that would of course result in double code and I would constantly have to update the fake class when a new method is introduced. So not perfectly.
  3. I can build up a fake implementation by using reflection and ExpandoObject, and just do nothing or return null when the particular method is called.

And the last option is what I am now after:

private static void SetupEcommerceLogic(IServiceCollection services, bool enabled)
{
    if (enabled)
    {
        services.AddTransient<IOrderBusinessLogic, OrderBusinessLogic>();
        return;
    }
    dynamic expendo = new ExpandoObject();
    IOrderBusinessLogic fakeBusinessLogic = Impromptu.ActLike(expendo);
    services.AddTransient<IOrderBusinessLogic>(x => fakeBusinessLogic);
}

By using Impromptu Interface, I am able to successfully create a fake implementation. But what I now need to solve is that the dynamic object also contains all the methods (mostly properties not needed), but those ones are easy to add. So currently I am able to run the code and get up until the point it will call the OrderBusinessLogic, then it will, logically, throw an exception that the method does not exist.

By using reflection, I can iterate over all the methods within the interface, but how do I add them to the dynamic object?

dynamic expendo = new ExpandoObject();
var dictionary = (IDictionary<string, object>)expendo;
var methods = typeof(IOrderBusinessLogic).GetMethods(BindingFlags.Public);
foreach (MethodInfo method in methods)
{
    var parameters = method.GetParameters();
    //insert magic here
}

Note: For now directly calling typeof(IOrderBusinessLogic), but later I would iterate over all the interfaces within a certain assembly.

Impromptu has an example as follows: expando.Meth1 = Return<bool>.Arguments<int>(it => it > 5);

But of course I want this to be dynamic so how do I dynamically insert the return type and the parameters.

I do understand that a interface acts like a contract, and that contract should be followed, I also understand that this is an anti-pattern, but extensive research and negotiations have been done prior to reaching this point, for the result system we want, we think this is the best option, just a little missing piece :).

  • I have looked at this question, I am not really planning on leaving .dll's out, because most likely I would not be able to have any form of IOrderBusinessLogic usable within InvoiceBusinessLogic.
  • I have looked at this question, but I did not really understand how TypeBuilder could be used in my scenario
  • I have also looked into Mocking the interfaces, but mostly you would then need to define the 'mocking implementation' for each method that you want to change, correct me if I am wrong.
CularBytes
  • 9,924
  • 8
  • 76
  • 101
  • One more thought about that. I could say this is option 4: manage module presence in the layers above the business logic layer, i.e. don't call `UpdateInvoicePaymentStatus` if the eCommerce module is not enabled. Is it a suitable solution for you ? – Spotted Apr 05 '19 at 11:54
  • Actually this option could even be a better option than any other, if applicable (=not adding this feature to a large existing legacy code base). Handling such concern as high (in the abstraction layers) as possible, meaning at least above the business logic one, makes a lot of sense to me. – Spotted Apr 05 '19 at 11:55

2 Answers2

2

Even tough the third approach (with ExpandoObject) looks like a holy grail, I foster you to not follow this path for the following reasons:

  • What guarantees you that this fancy logic will be error-free now and at every time in the future ? (think: in 1 year you add a property in IOrderBusinessLogic)
  • What are the consequences if not ? Maybe an unexpected message will pop to the user or cause some strange "a priori unrelated" behavior

I would definitely go down the second option (fake implementation, also called Null-Object) even though, yes it will require to write some boilerplate code but ey this would offer you a compile-time guarantee that nothing unexpected will happen at rutime !

So my advice would be to do something like this:

private static void SetupEcommerceLogic(IServiceCollection services, bool enabled)
{
    if (enabled)
    {
        services.AddTransient<IOrderBusinessLogic, OrderBusinessLogic>();
    }
    else
    {
        services.AddTransient<IOrderBusinessLogic, EmptyOrderBusinessLogic>();
    }
}
Spotted
  • 4,021
  • 17
  • 33
  • Thanks for your answer Spotted, but I disagree, in one year from now, or a bit later, automated tests with the modules enabled for that specific client will tell very quickly if something is working or not. And don't forget about the development process: If I would, in your example, add a property then I would probably also need that property, at which point I will probably realize very quickly that I need to 'fake' it as well. – CularBytes Apr 05 '19 at 09:37
  • 1
    @CularBytes I get your point and it makes sense. However I still persist that it is _safer_ to use a Null-Object because of the fastest feedback (you **can't** even compile). Going down the third road would suppose many preconditions: **if** the code is correctly covered with automated tests, **if** these tests are run before you deploy to production, **if** you are notified about the failure, **if** you (or any other) dare to run the software locally after adding a small feature to verify that it really works (even though the feedback will still be longer than at compile-time), etc. – Spotted Apr 05 '19 at 09:45
  • @CularBytes There are a lot of arguments against this solution and I won't take the risk of implementing it **if** I was the decider. You've been warned. :) – Spotted Apr 05 '19 at 09:45
  • I get your point as well Spotted, it makes perfect sense, but I just don't feel like creating a lot of empty classes, it would drive me nuts for every time I have to add a method. Although they are 'empty' you still have to implement it or else it will throw a `NotImplementedException`, meaning you will have to return 0 or -1 when an `int` is the return type. But is 0 good? or is -1 bad? Doesn't have to be. Out of interest I am still looking for the requested solution, but for now I will go with my answer (where I can return nullalbe int, might want to improve the extension). – CularBytes Apr 05 '19 at 10:54
  • If I ever switch back to option 1 (and your answer) I will promise you to report back, I am curious how it would look like in 1 year from now. Not that hard to switch back anyway. Thinking of it: If I would switch back I would still be looking for a generic way to check what implementation is used, perhaps my extension is not that bad after all... – CularBytes Apr 05 '19 at 10:56
  • @CularBytes So far I understand you choosed to "denaturate" `IOrderBusinessLogic` in order to return nullable values instead of primitive types ? – Spotted Apr 05 '19 at 11:46
0

For as long as there is no other answer for the solution I am looking for, I came up with the following extension:

using ImpromptuInterface.Build;
public static TInterface IsModuleEnabled<TInterface>(this TInterface obj) where TInterface : class
{
    if (obj is ActLikeProxy)
    {
        return default(TInterface);//returns null
    }
    return obj;
}

And then use it like:

public void UpdateInvoicePaymentStatus(InvoiceModel invoice)
{
    _orderBusinessLogic.IsModuleEnabled()?.UpdateOrderStatus(invoice.OrderId);
   //just example stuff
   int? orderId = _orderBusinessLogic.IsModuleEnabled()?.GetOrderIdForInvoiceId(invoice.InvoiceId);
}

And actually it has the advantage that it is clear (in the code) that the return type can be null or the method won't be called when the module is disabled. The only thing that should be documented carefully, or in another way enforced, that is has to be clear which classes do not belong to the current module. The only thing I could think of right now is by not including the using automatically, but use the full namespace or add summaries to the included _orderBusinessLogic, so when someone is using it, it is clear this belongs to another module, and a null check should be performed.

For those that are interested, here is the code to correctly add all fake implementations:

private static void SetupEcommerceLogic(IServiceCollection services, bool enabled)
{
    if (enabled) 
    {
        services.AddTransient<IOrderBusinessLogic, OrderBusinessLogic>();
        return;
    }
    //just pick one interface in the correct assembly.
    var types = Assembly.GetAssembly(typeof(IOrderBusinessLogic)).GetExportedTypes();
    AddFakeImplementations(services, types);
}

using ImpromptuInterface;
private static void AddFakeImplementations(IServiceCollection services, Type[] types)
{
    //filtering on public interfaces and my folder structure / naming convention
    types = types.Where(x =>
        x.IsInterface && x.IsPublic &&
        (x.Namespace.Contains("BusinessLogic") || x.Namespace.Contains("Repositories"))).ToArray();
    foreach (Type type in types)
    {
        dynamic expendo = new ExpandoObject();
        var fakeImplementation = Impromptu.DynamicActLike(expendo, type);
        services.AddTransient(type, x => fakeImplementation);

    }
}
CularBytes
  • 9,924
  • 8
  • 76
  • 101
  • (Still trying to convince you :-)). So now you (or any other developer) have to remember to call `IsModuleEnabled()` before using some types + forced to make subsequent null checks (when using `int? orderId`). Too bad that your business logic (the most important part of your software) becomes cluttered with that. – Spotted Apr 05 '19 at 11:52
  • I admire your determination :). Either the business logic or the layer above it (as you described in the comment below the question) becomes 'cluttered' with deciding if I would call a certain method or not. As I mentioned in [1.]: In more complex logic's, it's not always easy to decide based on the return type of something (of another module) needs to be called. You could but then the `logic` of making those decisions are outside of the business logic, which you might want to re-use (of course). I think a null check here and there wouldn't hurt, most likely done anyway. Indeed I will have to – CularBytes Apr 05 '19 at 12:53
  • ...remember calling `IsModuleEnabled()`, but like I said already, I would have to do the checks somewhere at some point anyway. And also, this will be kept to a minimum, as long as possible I would try to keep the 'unit of work' together. – CularBytes Apr 05 '19 at 12:53
  • Just one more thing that comes to my mind (I hope you don't get annoyed, but I wish to challenge your design in the right and kind way). With your approach I hope that `OrderBusinessLogic` is easy to instanciate for the following reason: unit tests (I'm sure you have a bunch of them for your business logic, do you ?). – Spotted Apr 05 '19 at 14:33
  • In order to unit test `UpdateInvoicePaymentStatus` you cannot pass the "Impromptu" implementation because it would mess your business logic (it wouldn't with a Null-object implementation). So if instanciating `OrderBusinessLogic` is easy that's fine. If it's not, how do you consider to maintain your tests ? – Spotted Apr 05 '19 at 14:35
  • The DI registration is done per web application, unit tests is a separate project and would have it's own DI registration, no registration at all or Mocked. After all a "unit" test must test a single unit, so calling `UpdateOrderStatus` would be mocked anyway. I don't see a problem in that at all. Remember that `IOrderBusinessLogic` is still of, guess what, `IOrderBusinessLogic` type. The implementation is just different (similar what would happen with your solution). So apart from the extension, I don't need to know anything about `Impromptu` (in terms of references). – CularBytes Apr 05 '19 at 15:46
  • Lol I can even Mock the extension (would need to edit it) to make sure I only test code within the module! haha my answer becomes better and better! – CularBytes Apr 05 '19 at 15:46