Method Naming Convention

[JasinCole]

Just a quick question in general, but my OCD and long names just doesn't feel right. Which is the better approach and why do you prefer that approach?

public interface IAcpinvRepository : IRepository<Acpinv>
{
    Task<IEnumerable<Acpinv>> GetAccountsPayableByDate(DateTime date);
    Task<IEnumerable<Acpinv>> GetAccountsPayableByDateRange(DateTime start, DateTime end);
    Task<IEnumerable<Acpinv>> GetAccountsPayableByStatus(int status);
    Task<IEnumerable<Acpinv>> GetAccountsPayableByStatusDueDate(int status, DateTime start, DateTime end);
}

Or, a well documented overload approach

public interface IAcpinvRepository : IRepository<Acpinv>
{
    Task<IEnumerable<Acpinv>> GetAccountsPayable(DateTime date);
    Task<IEnumerable<Acpinv>> GetAccountsPayable(DateTime start, DateTime end);
    Task<IEnumerable<Acpinv>> GetAccountsPayable(int status);
    Task<IEnumerable<Acpinv>> GetAccountsPayable(int status, Datetime start, Datetime end);
}

 

[JasinCole]

I think I have decided to stick with the longer more descriptive names. But I changed AccountsPayable to Invoice, which technically is what they are of type DbSet<Acpinv>.

I'd still like to know when it is appropriate to use overload methods and when to avoid them. What's your methodology for determining your code semantics?

{
    Task<Acpinv?> GetInvoice(int id);
    Task<IEnumerable<Acpinv>> GetInvoicesByDate(DateTime date);
    Task<IEnumerable<Acpinv>> GetInvoicesByDateRange(DateTime start, DateTime end);
    Task<IEnumerable<Acpinv>> GetInvoicesByStatus(int status);
    Task<IEnumerable<Acpinv>> GetInvoicesByStatusDueDate(int status, DateTime start, DateTime end);
}

 

[Skydiver]

As an aside, part of the .NET naming convention is to not use abbreviations. So Acpinv should be something like AccountsPayableInvoice.

As another aside, do you really need to implement the Repository pattern? Recall that Entity Framework already implements the repository pattern. If the goal is to have methods that more specialized than what the default model exposes, you should implement a set of extension methods instead.

 

[JasinCole]

The Acpinv is so I can maintain some familiarity with the database. All tables are 6 characters long. All 130 of them. I was dealt that hand and I can’t change it.

I don’t know anything about extension methods. I don’t know if it’s worth the hassle at the moment to change things.

 

[Skydiver]

Entity Framework is an a ORM -- Object-Relatonal Mapper. An ORM is used so that your can write object oriented code that deals with domain objects and the ORM will take care of all the relations needed to make a relational database to work. If you are writing our object oriented code as if you were still talking directly to a relational database, then you might as well just be using sets of DTOs (data transfer objects) instead of entities (e.g. domain objects). If you are talking to DTOs, so might as well just use straight ADO.NET and not even bother with EF.

 

[Skydiver]

Also you can use friendly domain names, and tell EF what the underlying table name is:

DataAnnotations Table Attribute in Code-First

Learn about the [Table] DataAnnotation attribute in FE 6 and EF Core.

www.entityframeworktutorial.net

 

[JasinCole]

Entity Framework is an a ORM -- Object-Relatonal Mapper. An ORM is used so that your can write object oriented code that deals with domain objects and the ORM will take care of all the relations needed to make a relational database to work. If you are writing our object oriented code as if you were still talking directly to a relational database, then you might as well just be using sets of DTOs (data transfer objects) instead of entities (e.g. domain objects). If you are talking to DTOs, so might as well just use straight ADO.NET and not even bother with EF.

I don’t really know where you’re going with this, talking above my head now

 

[cjard]

The general notion that SD is getting at is that you should aim to avoid importing poor design decisions and practices from other domains into your C#. I understand why you've done it, but 130 tables that all have weird, barely readable 6 character names is a poor design decision that should be left in the database.

C# and EF are capable of managing the transition so that the C# names can be nice, readable, self explanatory and make the code read like a book; you tell EF that an AccountsPayableInvoice entity is stored in the ACPINV table, and you write nice C#, and EF will write ugly SELECT t0.* FROM ACPINV t0 queries

Same for things like JSON parsing; if your json looks_like_this orLkThs and you really want C# that LooksLikeThis OrLikeThis, then you can tell the json serializer that the LooksLikeThis property maps to looks_like_this etc. There is no need to pollute C# with the conventions of other places

If your database supports table aliasing then you could start to move away from the poor design decisions of old, shed that technical debt, make a set of nicely named aliases and use them so that old and new software work side by side, get rid of the old software that uses the 6 letter names, then rename the tables

--

On your original question, I think I'd use documented overloads for this case as the return type and processing approach seems to be generally the same. If the processing approach had some meaningful side effect or the method return type was different then perhaps I'd name differently, using the method name to draw attention to the processing alteration rather than relying on eg inference from parameter names

This is a personal thing for me; I'd be equally happy to use someone else's library that had names like yours without my OCD giving it too much attention

I did want to point out though that your argument names are perhaps poor. You've got "DateTime start" and "DateTime end" and it's the method name that makes it clear that this is used on the DueDate property but your overload version doesn't make it clear at all, so your documentation would have to say which date (date raised, date due, date paid etc) was relevant, and then you've created a lookup. If the passed variables were well named it would help:

GetInvoices(Status.Paid, startDueDate, endDueDate)

But I'd argue that we shouldn't rely on the external use to aid documenting the internal behavior of the method

Naming the parameters better, and using their labels when calling:

GetInvoices(Status.Paid, fromDueDate: st, toDueDateInclusive: ed)

 

[cjard]

I don’t know anything about extension methods

They're just static methods that you write in an easy-to-read (left to right) order but the compiler rearranges to be called in hard-to-read order (inside out)

If you had a set of methods:

class ThingProcessor{
   static Thing DoThis(Thing t)
   static Thing ThenDoThis(Thing t)
}

You'd maybe call them like:

    Thing x = new Thing();
    ThingProcessor.ThenDoThis(ThingProcessor.DoThis(x));

See how, if you want the Do/Then order you have to put the Do inside the Then? And you mention the class name all the time. And it's a mess?

Extension methods look nearly the same, they just have "this" before the first parameter:

static class ThingExtensions{
   static Thing DoThis(this Thing t)
   static Thing ThenDoThis(this Thing t)
}

You can still call them like always:

    ThingExtensions.ThenDoThis(ThingExtensions.DoThis(x));

But you can also write:

    x.DoThis().ThenDoThis();

The compiler will convert it to the inside-out form for you.

That's all an extension method is: a static method that looks like it is called on an instance but is rewritten to be a static call by the compiler, and just so happens to also make things easier to read and flow especially if created to support "fluent" style (ie return something that is mentioned in other extension method first parameters)

You've used them hundreds of times:

    someInvoices.Where(x => x.Amount > 1000).Select(...)

    //becomes
    Enunerable.Select(Enumerable.Where(invoices, x => x.Amount > 1000), ...)

Why have them? Well, they're just a separate static method that accepts an instance, but not part of an inheritance on that instance. You can "add methods" to any class, even sealed ones, using them and it makes for more readable code - it's an illusion, but so are most of the features added to C# since about version 2

 

[JasinCole]

Let's take a step back, If I drop the repository pattern then all my calls to the database are going to implemented using the chaining method. How does that make it more clear what I am doing?

Take this line of code

return await Srvc.Lgracts.GetAccountsByRecnumAsync(accounts.Select(act => act.Number).ToList());

turns into

return await _context.Lgracts
    .Where(e => ids.contain((int)act.Recnum))
    .ToListAsync());

Now obviously in this contrived example the Where clause is simple and the code is easy to reason about. But when I am matching based on 3 or more things then it becomes less readable.

Are you suggesting that I make an extension method?

static IQueryable<T> FilterByRecordNumbers(this IQueryable<T> query, List<int> recordNumbers)
{
    return query.Where(e => ids.contain((int)act.Recnum));
}

and use it like??

_context.DbSet<TEntity.FilterByRecordNumbers(accounts.Select(act => act.Number).ToList()).ToListAsync();

and if so that means I can use for any of the DbSet<TEntity> types in my database, because all database tables have a Recnum. Can you overload these methods as well? Say can I have (2)FilterByRecordNumber methods that accept different parameters. For instance a List<int> versus just a int?

 

[Skydiver]

Almost.

You can make the this parameter more type specific so that you are not dependent on using generics, and you could also call ToListAsync() like you did in your original repository version.

 

[cjard]

Another thing for your naming conventions, btw, is to make awaitable methods have a ...Async suffix. The method doesn't have to be marked async; generally we suffix methods thus when they behave in an asyncronous manner. A non async method that returns an awaitable Task would be an example

 

[cjard]

Say can I have (2)FilterByRecordNumber methods that accept different parameters. For instance a List<int> versus just a int?

They can be overloaded, but I'd propose they should have different names (plurality of the word "number"). In this specific case you could also just pass an array of one to the sole method that filters by numbers