HiveBrain v1.2.0
Get Started
← Back to all entries
patternsqlMinor

Luhn algorithm MS-SQL

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
codereviewsqlsql-servervalidationstackoverflowfinance
sqlalgorithmluhn

Problem

This a solution Luhn algorithm for credit validation.

This algorithm is a basic one inspired by wikipedia. I mean no check for length, type, and so on. You can enhance the base code as you want it to be.

The listing code of the following fucntion is tested on MSSQL 2012:

IF OBJECT_ID('dbo.LuhnALGValidation') IS NOT NULL
BEGIN
    DROP FUNCTION dbo.LuhnALGValidation
    IF OBJECT_ID('dbo.LuhnALGValidation') IS NOT NULL
        PRINT '>>'
    ELSE
        PRINT '>>'
END
go
CREATE FUNCTION dbo.LuhnALGValidation
(
@Luhn VARCHAR(8000)
)
RETURNS BIT
AS
BEGIN
IF @Luhn LIKE '%[^0-9]%'
RETURN 0
DECLARE @Index SMALLINT,
@Multiplier TINYINT,
@Sum INT,
@Flag INT,
@temp TINYINT,
@Plus TINYINT
SELECT @Index = LEN(@Luhn),
@Multiplier = 1,
@Flag = 0,
@Sum = 0
WHILE @Index >= 1
BEGIN
    SELECT @temp = CAST(SUBSTRING(@Luhn, @Index, 1) AS TINYINT)
    IF @Flag = 1 
    BEGIN
        SELECT @temp = 2 * CAST(SUBSTRING(@Luhn, @Index, 1) AS TINYINT)
        IF @temp > 9 
        BEGIN
            SELECT @temp = @temp - 9
        END
    END

    SELECT @Sum = @Sum + @temp

    IF @Flag = 1 
        SELECT @Flag = 0 
    ELSE 
        SELECT @Flag = 1 
    SELECT @Index = @Index - 1
END 
RETURN CASE WHEN @Sum % 10 = 0 THEN 1 ELSE 0 END
END
go
IF OBJECT_ID('dbo.LuhnALGValidation') IS NOT NULL
    PRINT '>>'
ELSE
    PRINT '>>'
go


I want it to work in database side. No matter in performance.

If you can help me to enhance this code.

Solution

Checking if the input is valid

You use the pattern ` to check if the input contains any non numeric characters and then return 0 if it does. There is a better way to do this, use the ISNUMERIC() function.

VARCHAR(8000)?

Your input allows 8000 characters, which seems like more than you are likely to need. I would consider lowering this to a value that makes a little more sense. I can't see you ever needing to validate numbers over a hundred characters in length. Feel free to correct me if I am wrong about this though, I don't know what kind of data you intend to work with.

Changing @x = @x + y to @x += y

I don't know if you are familiar with this syntax, but instead of doing:

SELECT @temp = @temp - 9


you can do:

SELECT @temp -= 9


which does exactly the same.

@Plus

You seem to have mistakenly left the @Plus variable in the query, you can delete it as it is not used.

A neat little trick

Here is a little tick to toggle the value of @Flag between 1 and 0:

SELECT @Flag = 1 - @Flag


This works because if @flag is one, it becomes this:

SELECT @Flag = 1 - 1


and if @Flag1 is zero then it becomes this:

SELECT @Flag = 1 - 0


Your IF Statement

Firstly, you set @temp, then if a condition is true, you set it to something else. This would be better expressed as an IF statement to set it if the condition is true, and an else to set it otherwise. This means that @temp is set at most twice in each iteration of the loop, as opposed to the current at most three times.

Speaking of IF's

I understand that leaving out the BEGIN/END where you can is tempting, but it makes it more difficult for people to understand what the query is actually doing. I would recommend including the BEGIN/END even for the smallest IF statements.

Setting multiple variables using SELECT

According to this page, and my own testing after reading, it is more efficient to use select to set multiple variables at once. If you choose to use the little trick above to set @Flag, you can reduce this:

SELECT @Sum = @Sum + @temp

IF @Flag = 1 
    SELECT @Flag = 0 
ELSE 
    SELECT @Flag = 1 
SELECT @Index = @Index - 1


to this:

SELECT 
    @Sum += @temp,
    @Flag = 1 - @Flag, --Trick to toggle @Flag between 1 and 0 
    @Index -= 1


The end result

I took the liberty of combining the declaring and initial setting of the variables:

CREATE FUNCTION dbo.LuhnALGValidation
(
    @Luhn VARCHAR(8000)
)
RETURNS BIT
AS
BEGIN

IF ISNUMERIC(@Luhn) = 0
BEGIN    
    RETURN 0
END

DECLARE 
    @Index SMALLINT = LEN(@Luhn),
    @Multiplier TINYINT = 1,
    @Sum INT = 0,
    @Flag INT = 0,
    @temp TINYINT;

WHILE @Index >= 1
BEGIN

    IF @Flag = 1 
    BEGIN
        SELECT @temp = 2 * CAST(SUBSTRING(@Luhn, @Index, 1) AS TINYINT)
        IF @temp > 9 
        BEGIN
            SELECT @temp -= 9
        END
    END
    ELSE
    BEGIN
        SELECT @temp = CAST(SUBSTRING(@Luhn, @Index, 1) AS TINYINT)
    END

    SELECT 
        @Sum += @temp,
        @Flag = 1 - @Flag, --Trick to toggle @Flag between 1 and 0 
        @Index -= 1
    END 

    RETURN CASE WHEN @Sum % 10 = 0 THEN 1 ELSE 0 END
END
GO

Code Snippets

SELECT @temp = @temp - 9
SELECT @temp -= 9
SELECT @Flag = 1 - @Flag
SELECT @Flag = 1 - 1
SELECT @Flag = 1 - 0

Context

StackExchange Code Review Q#81966, answer score: 5

Revisions (0)

No revisions yet.