John H
John H

Reputation: 14655

Correctly generating a unique invoice id

I've been asked to clean up someone else's controller code, which generates an invoice, and I've run into something I don't know how to fix. The code in question is as follows (this is using EF 6: Code First):

var invid = db.TransportJobInvoice.Where(c => c.CompanyId == CompanyId)
                .Max(i => i.InvoiceId);
var invoiceId = invid == null ? 1 : (int)invid + 1;

The code is supposed to generate an invoiceId based on the company the invoice is being created for. So a small table of this might look as follows:

------------------------------
| Id | CompanyId | InvoiceId |
------------------------------
|  1 |         1 |         1 |
------------------------------
|  2 |         1 |         2 |
------------------------------
|  3 |         1 |         3 |
------------------------------
|  4 |         2 |         1 |
------------------------------
|  5 |         2 |         2 |
------------------------------

As you can see, the invoiceId would be generated based on the current number of invoices for the company in question. However, I think it's reasonable to suggest that two threads could execute the query before this line is evaluated:

var invoiceId = invid == null ? 1 : (int)invid + 1;

which would result in the same invoiceId being generated for two different invoices.

Is there a simple solution to this, possibly leveraging Entity Framework to do this automatically?

Upvotes: 1

Views: 5815

Answers (5)

HABO
HABO

Reputation: 15816

A rather different approach would be to create a separate table with the NextId for each CustomerId. As new customers are added you would add a new row to this table. It has the advantage that the numbers assigned to invoices can remain unique even if you allow deleting invoices.

create procedure GetInvoiceIdForCustomer
  @CustomerId as Int,
  @InvoiceId as Int Output
as
begin
  set nocount on

  begin transaction

  update CustomerInvoiceNumbers 
    set @InvoiceId = NextId, NextId += 1
    where CustomerId = @CustomerId

  if @@RowCount = 0
    begin
    set @InvoiceId = 1
    insert into CustomerInvoiceNumbers ( CustomerId, NextId ) values ( @CustomerId, @InvoiceId + 1 )
    end

  commit transaction
end       

end

Upvotes: 2

jhilden
jhilden

Reputation: 12419

I suggest using the identity for the primary key, very important!

I would then add a column for "CustomerInvoiceID" and put a compound unique key on CustomerID and CustomerInvoiceID".

Then, create a stored procedure that will populate the field CustomerInvoiceID after it has been inserted, here is some pseudo code:

CREATE PROCEDURE usp_PopulateCustomerInvoiceID 
    @PrimaryKey INT, --this is your primary key identity column
    @CustomerID INT
AS
BEGIN
    SET NOCOUNT ON;

    DECLARE @cnt INT;

    SELECT @CNT = COUNT(1)
    FROM TBL 
    WHERE CustomerID = @CustomerID
        AND PrimaryKeyColumn <= @PrimaryKey

    UPDATE tbl
    SET CustomerInvoiceID = @cnt + 1
    WHERE PrimaryKeyColumn = @PrimaryKey
END

Upvotes: 3

Jordy Langen
Jordy Langen

Reputation: 3591

I don't know if you can make the invoice id auto generated unless it's beinng threated as a foreign key (which I think it isn't).

You problem with multiple threads could be solved using a lock statement.

lock (myLock)
{
     var invid = db.TransportJobInvoice.Where(c => c.CompanyId == CompanyId)
            .Max(i => i.InvoiceId);
     var invoiceId = invid == null ? 1 : (int)invid + 1;
}

This will guarantee that only thread is executing these statements.

Be careful though, this could cause performance issues when those statements are executed alot in parallel and the query takes some significant time to execute.

Upvotes: 1

mbeckish
mbeckish

Reputation: 10579

Two possibilities:

Server-side: Don't compute the max(ID)+1 on the client. Instead, as part of the INSERT statement, compute the max(ID)+1, via an INSERT..SELECT statement.

Client-side: Instead of an incrementing int, generate a GUID on the client, and use that as your InvoiceID.

Upvotes: 2

David Crowell
David Crowell

Reputation: 3813

If you use an Identity field in SQL Server, this will be handled automatically.

Upvotes: 1

Related Questions