Static Data Helper for an Oracle Backend

Question

What I would like to do is have a method call like:

Database.Modify.InsertOrUpdate<ADDRESS>(address,
() =>
{
    var result = ctx.ADDRESS.Where(o => o.JURIDICAL_ADDRESS == "Y")
        .OrderByDescending(o => o.ID)
        .FirstOrDefault();
    return (result != null ? result.ID : 0);
},
() => ctx.ADDRESS.Add(address),
() => address.ID = resultId.Value);

and generic method like:

public static bool InsertOrUpdate<TTable>(ADDRESS address, Func<decimal?> exist, Action insert, Action update)
{
    decimal? resultId = null;
    Helper.Console.Try(()=> resultId = exist.Invoke(),
        string.Format("Finding ({0}) information ...", table.GetType()),
        string.Format("Finding ({0}) information done", table.GetType()),
        string.Format("Finding ({0}) information failed", table.GetType())
    );
    if( resultId == null || resultId == 0 )
    {
        return Helper.Console.Try(insert,
            string.Format("Adding ({0}) information in local context ...", table.GetType()),
            string.Format("Adding ({0}) information in local context done", table.GetType()),
            string.Format("Adding ({0}) information in local context failed", table.GetType())
        );
    }
    return Helper.Console.Try(update,
        string.Format("Updateing ({0}) information in local context ...", table.GetType()),
        string.Format("Updateing ({0}) information in local context done", table.GetType()),
        string.Format("Updateing ({0}) information in local context failed", table.GetType())
    );
}

At the moment I have :

public static bool InsertOrUpdate(ADDRESS address, Entities ctx, KnowledgeBase knowledgeBase)
{
    decimal? resultId = null;
    Helper.Console.Try(() =>
        {
            var result = ctx.ADDRESS.Where(o => o.JURIDICAL_ADDRESS == "Y")
                .OrderByDescending(o => o.ID)
                .FirstOrDefault();
            resultId = (result != null ? result.ID : 0);
        },
        "Finding (ADDRESS) information ...",
        "Finding (ADDRESS) information done",
        "Finding (ADDRESS) information failed"
    );
    if( resultId == null || resultId == 0 )
    {
        return Helper.Console.Try(() => ctx.ADDRESS.Add(address),
            "Adding (ADDRESS) information in local context ...",
            "Adding (ADDRESS) information in local context done",
            "Adding (ADDRESS) information in local context failed"
        );
    }
    return Helper.Console.Try(() => address.ID = resultId.Value,
        "Updateing (ADDRESS) information in local context ...",
        "Updateing (ADDRESS) information in local context done",
        "Updateing (ADDRESS) information in local context failed"
    );
}

And

public static bool InsertOrUpdate(COMPANY company, Entities ctx, KnowledgeBase knowledgeBase)
{
    decimal? resultId = null;
    Helper.Console.Try(() =>
        {
            resultId = knowledgeBase.Company[company.REGISTER_CODE];
        },
        "Finding (COMPANY) information ...",
        "Finding (COMPANY) information done",
        "Finding (COMPANY) information failed"
    );
    if( resultId == null || resultId == 0 )
    {
        return Helper.Console.Try(() => ctx.COMPANY.Add(company),
            "Adding (COMPANY) information in local context ...",
            "Adding (COMPANY) information in local context done",
            "Adding (COMPANY) information in local context failed"
        );
    }
    return Helper.Console.Try(() => company.ID = resultId.Value,
        "Updateing (COMPANY) information in local context ...",
        "Updateing (COMPANY) information in local context done",
        "Updateing (COMPANY) information in local context failed"
    );
}

Try example:

[DebuggerStepThrough]
public static bool Try(Action a, string t1, string t2, string t3)
{
    try
    {
        var text1 = String.Format("{0}\t{1}", DateTime.Now.ToString(Configuration.Dateformat), t1);
        var text2 = String.Format("{0}\t{1}", DateTime.Now.ToString(Configuration.Dateformat), t2);

        System.Console.ForegroundColor = ConsoleColor.Green;
        System.Console.WriteLine(text1);
        System.Console.ForegroundColor = ConsoleColor.White;
        a();
        System.Console.ForegroundColor = ConsoleColor.Green;
        System.Console.WriteLine(text2);
        System.Console.ForegroundColor = ConsoleColor.White;
        return true;
    }
    catch (Exception ex)
    {
        var text3 = String.Format("{0}\t{1} {2}", DateTime.Now.ToString(Configuration.Dateformat), t3, ex.StackTrace);

        System.Console.ForegroundColor = ConsoleColor.Red;
        System.Console.WriteLine(text3);
        System.Console.ForegroundColor = ConsoleColor.White;
        SaveDataFile(text3);
        return false;
    }
}

Edit

After reading Eric Lippert answer to a related problem, I though that I was ignoring the KISS principle. Therefore I stopped trying to find a way to write this.

ADDRESS queryResult = null;

// ReSharper disable RedundantArgumentName
updated = (Ui.Instance.Try(
    process: () => queryResult = result.ADDRESS
        .Where(o => o.JURIDICAL_ADDRESS == "Y")
        .OrderByDescending(o => o.ID)
        .FirstOrDefault(),
    onStart: Configuration.FtInfo1Address,
    onAfter: Configuration.FtInfo2Address,
    onError: Configuration.FtInfo3Address
    ) && (queryResult == null || queryResult.ID == 0)
    ? Ui.Instance.Try(
        process: () => ctx.ADDRESS.Add(address),
        onStart: Configuration.AtInfoilc1Address,
        onAfter: Configuration.AtInfoilc2Address,
        onError: Configuration.AtInfoilc3Address
        )
    : Ui.Instance.Try(
        process: () => { address.ID = queryResult.ID; },
        onStart: Configuration.UtInfoilc1Address,
        onAfter: Configuration.UtInfoilc2Address,
        onError: Configuration.UtInfoilc3Address
        ));
// ReSharper restore RedundantArgumentName

Just for completeness, this is what I would use in Ui class.

public class Ui
{
    //default value
    public static IUserInterface Instance
    {
        get
        {
            switch (Configuration.UItype)
            {
                case Configuration.UIconsole:
                    return ConsoleUi.Instance;
                //...
                default:
                    return ConsoleUi.Instance;
            }
        }
    }

    public interface IUserInterface
    {
        bool Try(Action process, string onStart, string onAfter, string onError, bool isCritical = Configuration.ThrowErrorsDefault);
        //...
    }
    public sealed class ConsoleUi : IUserInterface
    {
        ConsoleUi() { }
        public static IUserInterface Instance { get { return Nested.Instance; } }
        class Nested
        {
            static Nested() { }
            internal static readonly ConsoleUi Instance = new ConsoleUi();
        }
        [DebuggerStepThrough]
        public bool Try(Action process, string onStart, string onAfter, string onError, bool isCritical)
        {
            try
            {
                var text1 = String.Format("{0}\t{1}", DateTime.Now.ToString(Configuration.Dateformat), onStart);
                var text2 = String.Format("{0}\t{1}", DateTime.Now.ToString(Configuration.Dateformat), onAfter);

                Console.ForegroundColor = ConsoleColor.Green;
                Console.WriteLine(text1);
                Console.ForegroundColor = ConsoleColor.White;
                process();
                Console.ForegroundColor = ConsoleColor.Green;
                Console.WriteLine(text2);
                Console.ForegroundColor = ConsoleColor.White;
                return true;
            }
            catch (Exception ex)
            {
                var text3 = String.Format("{0}\t{1} {2}", DateTime.Now.ToString(Configuration.Dateformat), onError, ex.StackTrace);

                Console.ForegroundColor = ConsoleColor.Red;
                Console.WriteLine(text3);
                Console.ForegroundColor = ConsoleColor.White;
                Helper.SaveDataFile(text3);
                if (isCritical)
                    throw;
                return false;
            }
        }
        //...
    }
}

Remarks:

I can swap default UI during runtime
I can use precompiler to throw errors when i need them, but current approach is even better. Note: interface gives optional parameter default value, that it reads from conf.
I have ability to add single breakpoint to debug all errors (from there I can move up the call stack)
Argument names do improve the readability a little
Unique error codes are better then dynamically generated as devaloper can trace with ctrl+f
[DebuggerStepThrough] will remove extra overhead calls from custom tests.

Ocelot20 · Accepted Answer · 2014-06-16 17:12:17Z

The code itself is generally pretty clean looking, but I think it suffers in a few areas:

InsertOrUpdate screams: "one method that is concerned with two distinct things". I see InsertOrUpdate as an anti-pattern.
Coupling between Console/Output writing and persistence logic is too tight.
The use of Func/Action seems unnecessary and overly complex. Their use doesn't really constrain the use of this method to the intended purpose.
Catching all exceptions at this depth in the code is probably not a good idea.

On #1: I see InsertOrUpdate type methods a lot. It seems straightforward enough to combine them now, but as the logic grows, you will eventually wish you didn't have to check a whole bunch of conditions to see which operation you actually want to do. You can already see this seeping into your code by the fact that you have an exist lambda to give a place for this additional logic.

The problem is usually with code further up the call stack. What is your application doing that you don't already know if this is an insert or update operation? This will lead to additional ambiguity elsewhere in your code when other developers aren't sure which is supposed to be happening in certain cases. What if I have a method that I know is only capable of an insert? Now I'm forced to use a method called InsertOrUpdate when I only need Insert (and its dangerous to open this method up to Update operations.

On #2: This is similar to #1 in that you have a method that is responsible for multiple things (persistence and writing to a console). What if I want to use this method but the Console is not available? What if I want to write to a different log? Any changes to the console writing have an impact on persistence logic, which is not desirable. If you refactor this to separate these concerns into their own methods, you will be able to change them independently.

On #3:

Database.Modify.InsertOrUpdate(address,
() => return -200D;
() => MakeAPie(),
() => HireAClown());

See what I did there? This method is overly generic. Is the exist Func ever going to compare on anything else than id? Entity Framework relies on unique keys for all Entities, so you could just use that framework and take the check out of the callers hands entirely.

On #4: By catching all exceptions, you are not letting callers properly handle certain cases (or any case really). It's taking away their ability to listen for certain exceptions and handle them. Instead, they just get a general "something bad happened, but I can't really tell what it was or if it was even related to the Insert or `Update calls.

Edit - In response to your comments:

On #2: I'm not surprised that you can swap out the implementation easily, but writing functions that have a single responsibility is also a good practice for a number of other reasons, such as:

Easier to understand. Looking at your method signature, a developer would have no clues about the fact that it even does any logging.
Easier to test. When testing a method with a well defined single purpose, it's easier to test for the expected result. Baking in this additional functionality makes an external call that doesn't have to do with the method, and now we have to test that functionality as well. We also can't supply a test logger in this case.
Single location to diagnose problems or make changes. Decided not to use the Helper.Console class in favor of something else? Well now you have to change your persistence code when there really wasn't a need to. Save method fails? New developer doesn't have to look at Console writing code and can easily get to the meat of the problem (Helper.Console can throw its own exceptions too!). etc. etc.

On #4: There's a difference between DbSaveException (made that up) and an OutOfMemoryException. You've removed the applications ability to make the distinction here. Perhaps you would stop processing records if you couldn't connect to the db at all? Or maybe fail gracefully in the face of a full output log. EF tends to throw exceptions of only a few types related to db saving. At any rate, you could move this exception logging a step up in the stack and have it no longer be a direct concern of the Insert and Update methods.

I will note that a lot of these suggestions can be taken to extremes. If you're writing a throwaway application, or one that is so small it will take another developer a minute or so to get acclimated to your design choices, then some of this can be chalked up to YAGNI (extensive exception handling of edge cases, for example). There is a benefit to developing with these things in mind though, because in reality applications are changed frequently and get maintained by a number of different people. Some of these have a low cost of implementation so it's often a no-brainer to keep things loosely coupled and concise. In the end it takes experience to decide when you're going to be wasting time by doing everything perfectly (I know I end up over-thinking things sometimes) or getting it done.

#Ocelot20 Program is fully automatic and maps new data provider to existing system. (1) agree (removed method) - however soft {edit, delete} are implemented in database tier and InsertOrUpdate denotes that current instance will not cause archiving. (2) Not the case - I can change 1 line of conf to log this in DB, both or none. (3) Agree, removed. (4) Not the case - there are numerous constraints in database that will throw an error. If it's not data provider program will continue (it is a feature). Log info contains more then enough info to debug. — Margus, Jun 16 '14 at 16:14
(2) If I need single purpose methods (for testing) I can have NoUi with ThrowErrorsDefault = true. (4) Cosmetic problem caused by oversimplified example. I edited article to remove some of it. — Margus, Jun 17 '14 at 16:47

Mat's Mug · Answer 2 · 2014-06-16 14:43:26Z

You rely a lot on static classes and helpers.

Database.Modify.InsertOrUpdate<ADDRESS>(address,
() =>
{
    var result = ctx.ADDRESS.Where(o => o.JURIDICAL_ADDRESS == "Y")
        .OrderByDescending(o => o.ID)
        .FirstOrDefault();
    return (result != null ? result.ID : 0);
},
() => ctx.ADDRESS.Add(address),
() => address.ID = resultId.Value);

One problem with this approach, is that you're tightly coupling your components: any service (/object) that's using methods in the Database.Modify static class, are tied to that specific implementation, which means you can't possibly call a service's method without hitting the database - writing automated unit tests for your code is not going to be possible.

Even if it were, another problem is that since the static methods live at the type level, just about anyone, anywhere, can have a dependency on these static helpers, and there's no way to tell from the outside, since the services that depend on these helpers will just use them, as opposed to using a constructor-injected instance that tells you right away (just by looking at the constructor) what the dependencies are.

For example:

public class SomeService
{
    public void SaveEntities(SomeEntity entity)
    {
        Database.Modify.InsertOrUpdate<SomeEntity>(...);
    }
}

Compare to:

public class SomeService
{
    private readonly IUnitOfWork _unitOfWork;
    public SomeService(IUnitOfWork unitOfWork)
    {
        _unitOfWork = unitOfWork;
    }

    public void SaveEntities(SomeEntity entity)
    {
        _unitOfWork.InsertOrUpdate(entity);
        _unitOfWork.Commit();
    }
}

The difference is that the 2nd snippet tells you that it's working off an IUnitOfWork (an abstraction for something that encapsulates a database transaction), right off the bat from its constructor - and if you want you can inject a "fake" implementation of the IUnitOfWork interface to run unit tests that will not hit the database.

It's best to depend on abstractions, not on implementations to write maintainable, loosely-coupled and cohesive code. An implicit static dependency is pretty much the worse that could happen to any class, as far as testability goes.

Your Try signature could use much more descriptive parameter names:

public static bool Try(Action a, string t1, string t2, string t3)

How about this?

public static bool Try(Action action, string onStart, string onSuccessful, string onError)

However looking at how it's used...

return Helper.Console.Try(() => company.ID = resultId.Value,
    "Updateing (COMPANY) information in local context ...",
    "Updateing (COMPANY) information in local context done",
    "Updateing (COMPANY) information in local context failed"
);

It looks like you could just have 3 resource strings saying ..., done and failed, and then you could change your signature for this:

public static bool Try(Action action, string description)

..appending the appropriate resource string to the description as you go.

Might not be what you'd like to hear, but I would recommend to scratch that, and abstract the DbContext into an IUnitOfWork interface, and work off that - keep it simple, and make it testable. C# is an object-oriented language, but coding everything in static classes makes your code become procedural - you're not creating objects, you're just calling procedures and you depend on static types and procedures, when you should be depending on abstractions and objects.

#Mat's Mug (1) yes - In case of this sample code mocking database to write test would be hard. Real code contains quirks and features - so I did not copy / paste. (2) Real code uses context dependency injection (java example: Google Guice). (3) Log description is my version of "lorem ipsum dolor sit amet", real error messages are loaded from conf. (4) true - I generated Try method for this example with ReSharper left variables (5) As I use dependency injection I can use static when I don't need a 'new' instance. — Margus, Jun 16 '14 at 16:46
For example KnowledgeBase is an object, but creating it as static would not be efficient. At runtime when I get list of company register codes that I can use to build cache only for them compared to all companies. Anyways I do like how you found quite a few bugs that I totally ignored. You deserve +1 — Margus, Jun 16 '14 at 16:50
Halfway DI isn't DI, it's something else in-between - you're completely losing the benefits of DI (testability, for one) if you're relying on static helpers that are themselves a dependency somewhere. If you need a Singleton instance, you can easily configure your favorite IoC container to give you a Singleton instance, instead of writing static code. — Mat's Mug, Jun 16 '14 at 16:52
"Only a Sith deals in absolutes" stackoverflow.com/questions/241339/… — Margus, Jun 16 '14 at 17:13
Absolutely true: "Of course, there are exceptions to my dislike of static methods. True utility classes that do not pose any risk to bloat are excellent cases for static methods - System.Convert as an example. If your project is a one-off with no requirements for future maintenance, the overall architecture really isn't very important - static or non static, doesn't really matter - development speed does, however." - I completely agree with the entirety of this excellent post, thanks for sharing! — Mat's Mug, Jun 16 '14 at 17:23

asked	9 months ago
viewed	353 times
active	9 months ago

current community

your communities

more stack exchange communities

Static Data Helper for an Oracle Backend

Edit

2 Answers 2

Your Answer

Not the answer you're looking for? Browse other questions tagged c# entity-framework generics or ask your own question.

Hot Network Questions

current community

your communities

more stack exchange communities

Static Data Helper for an Oracle Backend

Edit

2 Answers 2

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged c# entity-framework generics or ask your own question.

Related

Hot Network Questions