sharparchitecture / Sharp-Architecture

S#arp Architecture: ASP.NET MVC Best Practices with NHibernate
http://sharparchitecture.github.io/
Other
593 stars 153 forks source link

Extend the Check class to allow specification of exception types and strongly typed argument references #73

Closed sandord closed 8 years ago

sandord commented 12 years ago

I wrote an implementation of this a few years ago and I've been using it in all my projects ever since.

Here's an example of what you can do with it:

Check.Require<ArgumentNullException>(documentRepository != null, () => documentRepository);
Check.Require<ArgumentOutOfRangeException>(companyId > 0, () => companyId);

The second argument is a lambda expression that refers to the argument (instead of the usual string literal) which makes it refactoring friendly.

seif commented 12 years ago

+1

Beats having a hard coded string.

seif commented 9 years ago

@sandord got the code fore this? :)

seif commented 9 years ago

Maybe replace the check class with.net's code contracts

sandord commented 9 years ago

As far as I remember, .net's code contracts require a Visual Studio addin that injects checking code during compilation for run-time checking and provides static checking as well. I'm not convinced using the code contracts will outweigh the cost. Besides, I haven't seen them being used in the wild and I don't get the impression there's a vivid community around the subject.

Anyway, it could work of course. I have updated the code I mentioned to work as a set of extension methods on ArgumentException. E.g.

if (name == null) throw new ArgumentNullException().For(() => name);

The implementation is not performing as well as specifying a string constant, at least when the exception is actually thrown. But I'm pretty sure no one should care about exception throwing performance too much because exceptions should never be a part of the normal execution path anyway. That's why they're called exceptions to begin with.

There are overloads for providing a message as well. I like this more than contracts because they're vanilla C#. I'm a little less inclined to use a framework for everything nowadays which I think is a good thing. Finally, tools such as ReSharper actively look for argument checks in your code and I don't think they are compatible with code contract libraries or hand rolled frameworks.

C# 6 will bring something that will make the mentioned extension methods obsolete:

if (name == null) throw new ArgumentNullException(nameof(name));

That's even better but we'll have to wait a while before C# 6 is released though so that's not an option for now.

cd21h commented 8 years ago

Mark as obsolete and use https://msdn.microsoft.com/en-us/library/system.diagnostics.contracts.contract(v=vs.110).aspx

cd21h commented 8 years ago

@seif Latest CC version fails to process FluentNHibernate.pdb jagregory/fluent-nhibernate#337

Also I'm not very happy with try/catch being generated for recursion checking

if (__ContractsRuntime.insideContractEvaluation <= 4)
{
  try
    {
      __ContractsRuntime.insideContractEvaluation++;
      __ContractsRuntime.Requires(type != null, null, "type != null");
    }
    finally
    {
      __ContractsRuntime.insideContractEvaluation--;
    }
}

and internal namespace System.Diagnostics.Contracts with exception and Contract checking code added to every assembly:

[CompilerGenerated]
internal static class __ContractsRuntime
{
    [Serializable]
    private sealed class ContractException : Exception
    { 
        ....
    }

I'd rather consider ReSharper command-line tools https://www.jetbrains.com/resharper/download/index.html#section=resharper-clt. There is a nuget package available https://www.nuget.org/packages/JetBrains.ReSharper.CommandLineTools/

seif commented 8 years ago

:s yeah I want a big fan of all the changes it made :s

Didn't know about reshape tools but they seem to be solving a different problem.

Seems like we have 2 options:

I prefer option one, but if I remember @shatl didn't like how reshaper doesn't recognize null checks. The code here isn't that complex and isn't difficult to maintain. On Sat, 6 Feb 2016 18:09 Vitaliy K notifications@github.com wrote:

@seif https://github.com/seif Latest CC version fails to process FluentNHibernate.pdb jagregory/fluent-nhibernate#337 https://github.com/jagregory/fluent-nhibernate/issues/337

Also I'm not very happy with try/catch being generated for recursion checking

if (ContractsRuntime.insideContractEvaluation <= 4) { try { ContractsRuntime.insideContractEvaluation++; ContractsRuntime.Requires(type != null, null, "type != null"); } finally { ContractsRuntime.insideContractEvaluation--; } }

and internal namespace System.Diagnostics.Contracts with exception and Contract checking code added to every assembly:

[CompilerGenerated] internal static class __ContractsRuntime { [Serializable] private sealed class ContractException : Exception { .... }

I'd rather consider ReSharper command-line tools https://www.jetbrains.com/resharper/download/index.html#section=resharper-clt

— Reply to this email directly or view it on GitHub https://github.com/sharparchitecture/Sharp-Architecture/issues/73#issuecomment-180825336 .

seif commented 8 years ago

Maybe mark as deprecated and comment to use https://github.com/ghuntley/conditions

On Sat, 6 Feb 2016 18:48 Seif Attar iam@seifattar.net wrote:

:s yeah I want a big fan of all the changes it made :s

Didn't know about reshape tools but they seem to be solving a different problem.

Seems like we have 2 options:

  • keep the check class. optionally add the functionality requested in this issue.
  • remove the check class and use hand rolled checks.

I prefer option one, but if I remember @shatl didn't like how reshaper doesn't recognize null checks. The code here isn't that complex and isn't difficult to maintain. On Sat, 6 Feb 2016 18:09 Vitaliy K notifications@github.com wrote:

@seif https://github.com/seif Latest CC version fails to process FluentNHibernate.pdb jagregory/fluent-nhibernate#337 https://github.com/jagregory/fluent-nhibernate/issues/337

Also I'm not very happy with try/catch being generated for recursion checking

if (ContractsRuntime.insideContractEvaluation <= 4) { try { ContractsRuntime.insideContractEvaluation++; ContractsRuntime.Requires(type != null, null, "type != null"); } finally { ContractsRuntime.insideContractEvaluation--; } }

and internal namespace System.Diagnostics.Contracts with exception and Contract checking code added to every assembly:

[CompilerGenerated] internal static class __ContractsRuntime { [Serializable] private sealed class ContractException : Exception { .... }

I'd rather consider ReSharper command-line tools https://www.jetbrains.com/resharper/download/index.html#section=resharper-clt

— Reply to this email directly or view it on GitHub https://github.com/sharparchitecture/Sharp-Architecture/issues/73#issuecomment-180825336 .

cd21h commented 8 years ago

https://github.com/ghuntley/conditions allocates validator for every condition validation which is not the best approach performance wise.

@seif I'll create another branch to add ReShaper annotations to S#Arch project and integrate R# command line tools so we can play with it.

cd21h commented 8 years ago

@seif I added R# annotations (design-time only) and code inspections to the build. Now we can see code inspections and duplicated code reports as appveyor build artifacts.