4
\$\begingroup\$

A little code I wrote that will check if the entered 4-digit number is Harshad or not (Harshad number is one which divided by the sum of its digits will leave no remainder, special Harshad is the same but will also work with flipped sum of digits, example - 1729/19 (19 is the sum of digits) and when flipped 1729/91 will leave no remainder as well).

I would love to see how it could become better (I'm still new so was limited with the commands I could've used).

; Harshad.asm - check if input from user (4 digits, 1000-9999) is a Harshad number and if so, check if it's a "special" Harshad number
;
    .MODEL SMALL
    .STACK 100h
    .DATA
RequestStr     DB 'Enter a 4 digit number (1000-9999):',13,10,'$'
IsHarshad      DB ' is a Harshad number.',13,10,'$'
SpecialHarshad DB 'It is also a special Harshad number.',13,10,'$'
NotHarshad     DB ' is not a Harshad number',13,10,'$'
IncorrectInput DB 13,10,'Input is incorrect.',13,10,'$'
Num            DW ? ;Will fit a "word" size (16 bits)
DigitSum       DW ? ;Sum of digits
TEN            DW 10
TENbyte        DB 10
Temp           DB ? ;Used to check if number is also special Harshad during the div process
Temp2          DB ? ;Used with special Harshad div process  
;
     .CODE
     MOV AX,@DATA                ;DS can be written to only through a register
     MOV DS,AX                   ;Set DS to point to data segment
     MOV AH,9                    ;Set print option for INT 21h
     MOV DX,OFFSET RequestStr    ;Set DS:DX to point to RequestString
     INT 21h                     ;Print RequestStr
;
NumberInput:
    ;First digit
    MOV AH,1        ;Set scanning (input) option for INT 21h
    INT 21h         ;Scan first digit
    MOV DX,0
    SUB AL,'0'      ;Converting from ascii value to numeral value
    CMP AL,1        ;First digit must be between 1 and 9 in order for the number to be of 4 digits
    JB WrongInput   ;Otherwise jump to WrongInput label
    CMP AL,9
    JA WrongInput
    MOV AH,0
    MOV DigitSum,AX ;Store only first digit's value at the variable DigitSum
    MUL TEN         ;Multiply AX by 10
    MOV Num,AX
    ;Second digit
    MOV AX,0
    MOV AH,1 
    INT 21h
    SUB AL,'0'
    CMP AL,0
    JB WrongInput
    CMP AL,9
    JA WrongInput
    MOV AH,0
    ADD DigitSum,AX ;Add only second's digit value to DigitSum
    ADD Num,AX      ;Add AX's value (which has been multiplied by 10 with the first digit) to Num variable
    MOV AX,0
    MOV AX,Num      ;Move new Num's value to AX to multiply it by 10
    MUL TEN
    MOV Num,AX
    ;Third digit
    MOV AX,0
    MOV AH,1
    INT 21h
    SUB AL,'0'
    CMP AL,0
    JB WrongInput
    CMP AL,9
    JA WrongInput
    MOV AH,0
    ADD DigitSum,AX
    ADD Num,AX
    MOV AX,0
    MOV AX,Num
    MUL TEN
    MOV Num,AX
    ;Forth digit
    MOV AX,0
    MOV AH,1 
    INT 21h
    SUB AL,'0'
    CMP AL,0
    JB WrongInput
    CMP AL,9
    JA WrongInput
    MOV AH,0
    ADD DigitSum,AX ;Now DigitSum contains the sum of each of the 4 digits in the number
    ADD Num,AX ;Num contains full 4 digits number
    JMP CheckHarshad    
WrongInput:      
    MOV AH,9
    MOV DX,OFFSET IncorrectInput
    INT 21h
    JMP CodeEnd
CheckHarshad:
    MOV AX,0
    MOV DX,0
    MOV AX,Num
    DIV DigitSum ;Number will be stored in AX and the remainder in DX
    CMP DX,0 ;Check if there is remainder or not
    JE CheckSpecialHarshad
    MOV AH,9
    MOV DX,OFFSET NotHarshad 
    INT 21h
    JMP CodeEnd
CheckSpecialHarshad:
    MOV AH,9
    MOV DX, OFFSET IsHarshad
    INT 21h
    MOV AX,0
    MOV AX,DigitSum
    DIV TENbyte
    MOV Temp,AL
    MOV Temp2,AH
    MOV AX,0
    MOV AL,Temp2
    MUL TENbyte
    ADD AL,Temp
    MOV Temp2,AL ;Temp2 now has the DigitSum number flipped
    MOV AX,0
    MOV AX,Num
    DIV Temp2
    CMP AH,0
    JNE CodeEnd
    MOV DX,OFFSET SpecialHarshad
    MOV AH,9
    INT 21h
CodeEnd:
    MOV AH,4Ch
    INT 21h
    END
\$\endgroup\$
1
\$\begingroup\$

Because your program is already way above average, I encouraged you to post it on Code Review. I'm glad you decided to do so. Next comes the review that I promissed to write.

A conceptual error

You've programmed a byte-sized division for the "special Harshad" test (div Temp2 along with Temp2 db ?). This is not OK! You need a word-sized division just like in the "normal Harshad" test.

Consider the number 4368. The sum of its digits is 21. The flipped sum of its digits is 12.
Division by 21 gives 208 but division by 12 gives 364. This quotient is bigger than AL can hold! It will generate a division exception #DE.

Powerful loops

There's a lot of duplicated code when you input the 4-digit number.
Where the extra constraint on the 1st digit could justify a separate step, inputting the next 3 digits should definitely go in a loop.

A better strategy however is to not check the validity of the input based on its digits but rather on the total value. For you know that any 4-digit number that is not allowed to start with a zero, will lie between 1000 and 9999.
Simply keep asking for a digit until the total value is no longer below 1000.


Bad redundancy vs good redundancy

You often wrote mov ax, 0 followed by another instruction that loads the AX register. That mov ax, 0 is not productive!

When the number checks out to be a special Harshad number, your program displays two separate messages. This is a redundancy since stating that a number is "special Harshad" already implies that it is "normal Harshad".

Testing for a valid digit you wrote CMP AL,0 JB WrongInput. This is a redundant operation since nothing can ever be below zero. Remember the below condition pertains to unsigned values. Using jl WrongInput does make sense in this case.

As a 1st example of good redundancy consider this:

        mov     si, BAD

        ; Input and test number
        ; Jump to Msg if invalid

        mov     si, NOK

        ; Test for Harshad
        ; Jump to Msg if not Harshad

        mov     si, OK1

        ; Test for special Harshad
        ; Jump to Msg if just Harshad

        mov     si, OK2

        ; Fall through if special Harshad

Msg:    mov     dx, si
        mov     ah, 09h                 ;DOS.PrintText
        int     21h

Loading the SI register with the address of the BAD message is redundant if the code that follows should determine that the number is fine.
If on the other hand that code finds the number to be invalid, we can jump readily to a single place in the code where the result is shown.

Loading the SI register with the address of the NOK message is redundant if the code that follows should determine that the number is indeed Harshad.
If on the other hand that code finds the number to be not Harshad, we can jump readily to a single place in the code where the result is shown.

Loading the SI register with the address of the OK1 message is redundant if the code that follows should determine that the number happens to be special Harshad. If on the other hand that code finds the number to be just Harshad, we can jump readily to a single place in the code where the result is shown.

As a 2nd example of good redundancy consider this:

        xor     bx, bx                  ;InputNumber [1000,9999]
Key:    mov     ax, 10
        mul     bx
        mov     bx, ax
        mov     ah, 01h                 ;DOS.GetKey
        int     21h                     ; -> AL

Isn't it silly to multiply by 10 when the number is known to be zero?
No, because this arrangement allows for cleaner/shorter code.
Had I placed the multiplication after the DOS.GetKey function, I would have had to move the keycode from AL to a spare register in order to do the multiplication that mandatory uses AX, at least on 8086.

On 80386 and better, the issue would not exist since then multiplying any register by an immediate is an option:

        cbw
        imul    bx, 10
        add     bx, ax

Useful shortcuts

At the very least the following changes will give you shorter code which is generally speaking a good thing:

From          To
----          --
mov  bx, 0    xor  bx, bx  If FLAGS are not important, or you need CF=0
cmp  dx, 0    test dx, dx
mov  ah, 0    cbw          Provided AL holds a positive number
mov  dx, 0    cwd          Provided AX holds a positive number

Explore the instruction set

aam divides the AL register by 10, leaves the quotient in AH and the remainder in AL.
aad multiplies the AH register by 10, adds AL and stores the result in AX.

Given these descriptions it should be clear that these are ideal instructions to flip the digits in the SumOfDigits value.

        mov     al, cl                  ;CX is SumOfDigits [1,36]
        aam
        xchg    al, ah
        aad
        mov     cl, al                  ; -> CX is flipped SumOfDigits

My version of the program

The ORG 256 directive means that I've assembled this into a program with the .COM extension. If you prefer the .EXE style you'll need to arrange the DATA and CODE sections appropriately, initialize the DS segment register, and also drop the ORG 256 directive.

        ORG     256

        mov     dx, ASK
        mov     ah, 09h                 ;DOS.PrintText
        int     21h

        mov     si, BAD
        xor     cx, cx                  ;SumOfDigits [1,36]
        xor     bx, bx                  ;InputNumber [1000,9999]
Key:    mov     ax, 10
        mul     bx
        mov     bx, ax
        mov     ah, 01h                 ;DOS.GetKey
        int     21h                     ; -> AL
        sub     al, "0"
        jb      Msg        <<< No need for a separate CMP here, since
        cmp     al, 9          SUB already defined the required FLAGS
        ja      Msg
        cbw
        add     bx, ax
        add     cx, ax
        cmp     bx, 1000
        jb      Key

        mov     si, NOK
        mov     ax, bx
        cwd
        div     cx
        test    dx, dx
        jnz     Msg

        mov     si, OK1
        mov     al, cl                  ;CX is SumOfDigits [1,36]
        aam
        xchg    al, ah
        aad
        mov     cl, al                  ; -> CX is flipped SumOfDigits
        mov     ax, bx
        cwd
        div     cx
        test    dx, dx
        jnz     Msg

        mov     si, OK2

Msg:    mov     dx, si
        mov     ah, 09h                 ;DOS.PrintText
        int     21h
        mov     ax, 4C00h               ;DOS.TerminateWithExitcode
        int     21h
; --------------------------------------
ASK:    db      'Please input a number between 1000 and 9999:', 13, 10, '$'
BAD:    db      ' is not a valid number.', 13, 10, '$'
NOK:    db      ' is not a Harshad number.', 13, 10, '$'
OK1:    db      ' is a Harshad number.', 13, 10, '$'
OK2:    db      ' is a special Harshad number.', 13, 10, '$'

        END

Testing the program on a variety of inputs:

An example of running the program


A cool trick

BAD:    db      ' is not a valid number.', 13, 10, '$'
NOK:    db      ' is not a Harshad number.', 13, 10, '$'
OK1:    db      ' is a Harshad number.', 13, 10, '$'
OK2:    db      ' is a special Harshad number.', 13, 10, '$'

Looking at these messages, you'll see they all end the same way. In their present form these identical endings consume 40 bytes. A space savings of 23 bytes is possible through separating and later re-combining.
This is a cool trick found in some of the larger DOS programs where wasting space is not an option!
I'm not saying that you should write it this way in your little demonstration program though...

Msg:    mov     dx, si
        mov     ah, 09h                 ;DOS.PrintText
        int     21h
        mov     dx, FUN
        mov     ah, 09h                 ;DOS.PrintText
        int     21h
        mov     ax, 4C00h               ;DOS.TerminateWithExitcode
        int     21h
; --------------------------------------
ASK:    db      'Please input a number between 1000 and 9999:', 13, 10, '$'
BAD:    db      ' is not a valid$'
NOK:    db      ' is not a Harshad$'
OK1:    db      ' is a Harshad$'
OK2:    db      ' is a special Harshad$'
FUN:    db      ' number.', 13, 10, '$'

An additional 4 byte savings is possible if we go crazy and apply the same principle on the identical beginnings of these messages (' is ').

\$\endgroup\$
  • \$\begingroup\$ Thank you so much for this incredible effort of yours to explain every single thing you did (even the ORG 256 explanation and using the space efficiently is a must, but not so much on this one :)). I've truly learned so much from this! The "TEST" command, if it's on a "BYTE" size it will work with ZF and check if the lower part is zero? and if it's "WORD" size then check for the sign of the digit? if so, you used it to see if there's any remainder left? And one more question is why it's better to use "CWD" instead of "MOV DX,0"? Thank you again, much appreciated!!! \$\endgroup\$ – S.Arkab Dec 9 '18 at 19:55
  • 1
    \$\begingroup\$ @S.Arkab When the TEST instruction operates on 2 identical register operands like in test bl, bl or test dx, dx, it means we compare the register with zero. TEST defines less flags than CMP, but there's no difference between the BYTE size or the WORD size other than that the signbit for the BYTE is bit 7 and the signbit for the WORD is bit 15. \$\endgroup\$ – Sep Roland Dec 9 '18 at 20:50
  • 1
    \$\begingroup\$ @S.Arkab The benefit of choosing CWD instead of MOV DX, 0 is that it is a 1-byte instruction where the MOV is a 3-byte instruction. As I mentioned in the review, they are equivalent only if the value in AX is positive (bit 15 of AX is zero). \$\endgroup\$ – Sep Roland Dec 9 '18 at 20:53
2
\$\begingroup\$

Written for NASM

Your program is a little hard to follow, mostly due to poor formatting. There is a lot of unnecessary duplication too and the user has no opportunity for correcting mistakes. The following prologue will address these initial criteria;

  1. Prompt operator for what is required.
  2. Use a function that allows the user to correct mistakes.
  3. Test that entry is 4 characters long and the first digit is not zero.
      org   0x100

      DOS   equ 21H
     SHOW   equ  9
 BUFFERED   equ 10  ; Buffered input
   OUTCHR   equ  2
   GETCHR   equ  8


Begin:  mov ah, SHOW        ; Function 9 display string terminated with '$'
        mov dx, Prompt
        int DOS

        mov ah, BUFFERED    ; Function 10 Get buffered input
        mov dx, Entry
        int DOS

; Test that your criteria of 4 character entry and first digit not zero has been met.

        cld                 ; Make sure SI auto-increments
        mov si, dx
        inc si              ; Point to number of characters entered
        lodsb
        or  al, al
        jz  Err             ; You might want to put some sort of prompting text here
        cmp al, 4   
        jnz Err
        cmp byte [si], '1'
        jae Evaluate

Err:    mov ah, SHOW        ; Function 9 again
        mov dx, ErrTxt
        int DOS
        ret                 ; Return to DOS

You do have these elements in your program, but I think you'll agree when there are in one place it's a lot easier to recognize which error testing mechanism has been implemented. Notice too, formatting is monumentally important, especially in ASSM.

Now we are ready to do the calculations and because we're not going to be using any DOS functions, there is a little more freedom in which registers we can use and for what. The criteria now becomes, establish a binary value of our entry and binary total of all digits.

Evaluate:

    ; AX = Working register, CX = character count

        movzx   cx, al      ; Move character count into CX
        xor     dx, dx      ; DX will be binary representation of input
        mov     bx, dx      ; BL will be total of digits
        mov     ax, dx

  .get: lodsb           ; Read character from input string
        cmp     al, '0'
        jb      Err
        cmp     al, '9'
        ja      Err

       and      ax, 1111b   ; Could be SUB AL,'0' too.
       add      bl, al      ; Bump total of digits
       xchg     ax, dx
       imul     ax, 10
       add      dx, ax
       loop     .get

     ; Now DX = Binary equivalent and BL divisor

        xor     ax, ax
        xchg    ax, dx
        push    ax          ; Going to need this again to determine if Special
        idiv    bx
        or      dl, dl      ; Determine if value is Harshad or not.

From this point I'd pretty much be doing the same as you have except there is still no need to address anything in memory other than prompting text as everything you need is on the stack and BL. Let us know what you come up with then we'll compare it with what I have.

\$\endgroup\$
  • \$\begingroup\$ I don't know some of the commands you used (but can guess what some might do, such as loop and xchg). I will study what you wrote and try to improve my code as you suggested. Thank you! \$\endgroup\$ – S.Arkab Dec 4 '18 at 10:57

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.