coins

[logicalaxiom]

i am supposed to make a program that give option of searching for coins or sorting them in alphabetical order i don't know what is wrong with this program any ideas? or sugeestions would be helpful :rotfl:

Code:

CLS
OPTION BASE 1
DIM coin$(10), prices(10)
CLS
GOSUB setup
GOSUB menu
GOSUB process
GOSUB shellsort
GOSUB prints
GOSUB wrapup
END
setup:
h1$ = "coin              price"
h2$ = "----              -----"
p1$ = "\                \#####"
RETURN
menu:                              
PRINT "                               Û±ÛÛ°Û±Û±Û"
PRINT "                               °Û MENU Û°"
PRINT "                               Û±ÛÛ°Û±Û±Û"
'FOR x = 100 TO 500 STEP 25
'SOUND x, 2
'SOUND 600 - x, 2
'NEXT x
PRINT "1) Sort"
PRINT "2) Search"
PRINT "3) End"
INPUT "choose an option===>"; menu
SELECT CASE menu
CASE 1
GOSUB shellsort
CASE 2
GOSUB processreport
CASE 3
END
CASE ELSE
PRINT "Invalid"
SLEEP 1
SLEEP 1
GOSUB menu
END SELECT
RETURN
process:
OPEN "coin.dat" FOR INPUT AS #1
FOR x = 1 TO 10
INPUT #1, coin$(x), prices(x)
NEXT x
CLOSE
RETURN
shellsort:
gap = number / 2
DO WHILE gap > 0
limit = number
switch$ = "on"
DO WHILE switch$ = "on"
switch$ = "off"
FOR i = 1 TO limit - gap
IF coin$(i) > coin$(i + gap) THEN
SWAP coin$(i), coin$(i + gap)
SWAP prices(i), prices(i + gap)
switch$ = "on"
last = i
END IF
NEXT i
limit = last
LOOP
gap = gap / 2
LOOP
RETURN

prints:
PRINT h1$
PRINT h2$
FOR x = 1 TO 10
PRINT USING p1$; coin$(x); prices(x)
total = total + prices(x)
NEXT x
RETURN
wrapup:
PRINT
PRINT USING p1$; "total = "; total
RETURN


processreport:
'DO
'GOSUB acceptoperator.input
'GOSUB accesstable.function
'LOOP UNTIL UCASE$(control$) = "N"
'GOSUB menu
'acceptoperator.input:
'CLS
'LOCATE 5, 15
'INPUT "Coin Name"; searcharguement
'GOSUB accesstable.function
'accesstable.function:
CLS
DO
FOR x = 1 TO 10
row% = CSRLIN
col% = POS(0)
coin$(x) = coin$(x) + w$
w$ = INPUT$(1)
IF w$ = CHR$(13) THEN EXIT DO
IF w$ = CHR$(8) THEN
IF NOT coin$(x) = "" AND NOT col% = 1 THEN
coin$(x) = MID$(coin$(x), 1, LEN(coin$(x)) - 1)
w$ = " "
LOCATE row%, (col% - 1): PRINT " ";
LOCATE row%, (col% - 1)
ELSE
w$ = ""
END IF
ELSE
PRINT "*";
END IF
PRINT coin$(x)
LOOP
PRINT
NEXT x
'FOR prices = 1 TO 10
'IF searcharguement$ = coin$ THEN
'EXIT FOR
'END IF
'NEXT prices
'IF number <= x THEN
'GOSUB displaytable.function
'ELSE
'LOCATE 7.15
'PRINT "Coin Name"; searcharguement; "Not Found"
'END IF
'LOCATE 11, 15
'INPUT "enter y to look up another Coin else n"; control$
'GOSUB displaytable.function
'displaytable.function:
'LOCATE 7, 15
'PRINT "Coin====>"; coin$(x)
'LOCATE 11, 15
'p1$ = "price===>####"
'PRINT USING p1$; price(x)
'GOSUB menu

my data file

Code:

"Super Nickle",14.25
"DoubleSided Quarter",10.50
"Europe's Currency",0.00
"Olympics Dime",5.00
"Random Penny",0.01
"Gold Quarter",10.00
"Victorian Pound",1.55
"Confederate Coin",1.00
"New Year Coin",5.00
"Birthday Coin",5.00[/code]

[Moneo]

Axiom,

I think people don't want to touch your program for the following reasons:
1) The code is not alligned, like the code between a FOR and a NEXT are in the same column. Hard to read.
2) There's commented-out code in several places. Get rid of it.

Ok, I tried to compile it and got some errors:

1) In the PROCESSREPORT subroutine, there's a DO and then a FOR up front, but the LOOP comes before the NEXT. It really looks like you don't need the DO and the LOOP.

2) This subroutine just falls off the end of the program. It's missing a RETURN.

3) Can't figure out what the subroutine does. The menu says SEARCH, but this doesn't look like a search.

Fix it up, compile it and test it. Then, if it doesn't do what you expected, post it again with an explanation.
*****

[logicalaxiom]

ok.. sorry about that i think i know what to do now. are there any rules for spacing before for or next ?

[Moneo]

Spacing within a FOR NEXT or a DO LOOP is usually something like this:

Code:

for x = 1 to 10
     y = x +1
     z = x - 1
     if x = 5 then
        print z
     end if
     rem ...etc.
next x

x = 1
do while x < 11
     y = x +1
     z = x - 1
     if x = 5 then
        print z
     end if
     rem ...etc.
     x = x +1
loop

*****

[Ralph]

logicalaxiom:
As Moneo, aka Na_th_an, shows, the only "rule" to follow is, "Make sure that your code is easy to read".

You indent (I usually use two spaces, four, etc,) all code that is subjugated, in some "reasonable" manner, use one blank line to separate loops, and two or three lines between mayor pieces of code. You might think of your code as a book, in which chapters, paragraphs, sentences and words are separated in such a manner as to make reading and understanding easier.

The best way to learn how to write clear and readable code is to look at various of the postings on this forum, and try to emulate those that seem well organized and easy to follow. Oh, yes, and use remarks sparingly, to introduce or describe what each section of code is to do, or to explain certain difficult to understand coding.

As an example of how I would use indenting and spaces, I quote some of your code, after my personal way of indenting/spacing, as follows:

Code:

'CoinSort, Version 1.0, May 16, 2005
'This is a coin search or sort program, by LogicAxiom

'preliminaries:
CLS
OPTION BASE 1
DIM coin$(10), prices(10)
CLS

'main program:
GOSUB setup
GOSUB menu
GOSUB process
GOSUB shellsort
GOSUB prints
GOSUB wrapup
END
'end of main program

.--------------------------------

'SUBs section:

setup:
h1$ = "coin              price"
h2$ = "----              -----"
p1$ = "\                \#####"
RETURN


menu:                              
PRINT "                               Û±ÛÛ°Û±Û±Û"
PRINT "                               °Û MENU Û°"
PRINT "                               Û±ÛÛ°Û±Û±Û"
'FOR x = 100 TO 500 STEP 25
'  SOUND x, 2
'  SOUND 600 - x, 2
'NEXT x
PRINT "1) Sort"
PRINT "2) Search"
PRINT "3) End"
INPUT "choose an option===>"; menu

SELECT CASE menu
  CASE 1
    GOSUB shellsort
  CASE 2
    GOSUB processreport
  CASE 3
    END
  CASE ELSE
    PRINT "Invalid"
    SLEEP 1
    SLEEP 1
   GOSUB menu
END SELECT

RETURN

[Moneo]

Axiom,

In the menu subroutine, when the selection is invalid you do a gosub menu. You really want to start the menu again, so do a goto menu.

The shellsort subroutine uses the variable called number, but number was never set to anything.
*****

[logicalaxiom]

Spacing within a FOR NEXT or a DO LOOP is usually something like this:

Code:

for x = 1 to 10
     y = x +1
     z = x - 1
     if x = 5 then
        print z
     end if
     rem ...etc.
next x

x = 1
do while x < 11
     y = x +1
     z = x - 1
     if x = 5 then
        print z
     end if
     rem ...etc.
     x = x +1
loop

*****

thanx i will do that in the future
i am finished my code is like this :

Code:

CLS
OPTION BASE 1
CLS
GOSUB setup
GOSUB menu
GOSUB shellsort
GOSUB prints
GOSUB wrapup
END
setup:
h1$ = "coin              price"
h2$ = "----              -----"
p1$ = "\                \#####"
RETURN
menu:                          
PRINT "                               旁栢겡글글"
PRINT "                               겡 MENU 方"
PRINT "                               旁栢겡글글"
PRINT "1) Sort"
PRINT "2) Search"
PRINT "3) End"
    INPUT "choose an option===>"; menu
SELECT CASE menu
     CASE 1
GOSUB shellsort
     CASE 2
GOSUB process
     CASE 3
END
CASE ELSE
PRINT "Invalid"
SLEEP 1
GOSUB menu
END SELECT

process:
CLS
COLOR 4
OPEN "coin.dat" FOR INPUT AS #1
FOR x = 1 TO 10
INPUT #1, coin$(x), price(x)
NEXT x
c = 10
INPUT "coin name"; s.s$
num = LEN(s.s$)
FOR x = 1 TO 10
pl = LEN(coin$(x))
FOR y = 1 TO pl - (num - 1)
IF s.s$ = LCASE$(MID$(coin$(x), y, num)) THEN
PRINT coin$(x), price(x)
END IF
NEXT y
NEXT x
CLOSE #1
INPUT "do you want to search again(y or n)"; yn$
IF UCASE$(yn$) = "Y" THEN
GOSUB process

ELSE
GOSUB wrapup
END IF

shellsort:
OPEN "coin.dat" FOR INPUT AS #1
FOR x = 1 TO 10
INPUT #1, coin$(x), price(x)
NEXT x
CLOSE #1
limit = 10
switch$ = "on"
DO WHILE switch$ = "on"
switch$ = "off"
FOR y = 1 TO (limit - 1)
IF coin$(y) > coin$(y + 1) THEN
SWAP coin$(y), coin$(y + 1)
SWAP price(y), price(y + 1)
switch$ = "on"
last = y
END IF
NEXT y
limit = last
LOOP
CLOSE #1
GOSUB prints

prints:
PRINT
PRINT h1$
PRINT h2$
FOR i = 1 TO 10
PRINT USING p1$; coin$(i); price(i)
NEXT i
GOSUB wrapup

wrapup:
PRINT "process completed"
END
RETURN

[SJ Zero]

I was bored, so I ran your code through fb.

I also removed the gosubs, because gosubs are for commies.

Code:

Declare Sub Setup ()
Declare Sub Menu ()
Declare Sub ShellSort ()
Declare Sub Prints ()
Declare Sub Wrapup ()
Declare Sub process ()

Dim Shared H1$, H2$, P1$
Dim Shared coin$(1 To 10), price(1 To 10)

'Main Loop starts here. Converted to subs because GOSUB is for commies
setup       ()
menu        ()
shellsort   ()
prints      ()
wrapup      ()



Sub setup
    h1$ = "coin              price"
    h2$ = "----              -----"
    p1$ = "\                \#####"
End Sub
Sub menu                          
    Print "                               /------\"
    Print "                               | MENU |"
    Print "                               \------/"
    Print "1) Sort"
    Print "2) Search"
    Print "3) End"
    Input "choose an option===>"; menuChoice
    Select Case menuChoice
    Case 1
        shellsort ()
    Case 2
        process ()
    Case 3
        End
    Case Else
        Print "Invalid"
        Sleep 1
        menu ()
    End Select
End Sub

Sub process
    Cls
    Color 4
    Open "coin.dat" For Input As #1
    For x = 1 To 10
        Input #1, coin$(x), price(x)
    Next x
    c = 10
    Input "coin name"; s.s$
    num = Len(s.s$)
    For x = 1 To 10
        pl = Len(coin$(x))
        For y = 1 To pl - (num - 1)
            If s.s$ = Lcase$(Mid$(coin$(x), y, num)) Then
                Print coin$(x), price(x)
            End If
        Next y
    Next x
    Close #1
    Input "do you want to search again(y or n)"; yn$
    If Ucase$(yn$) = "Y" Then
        process ()
        
    Else
        wrapup ()
    End If
End Sub

Sub shellsort
    Open "coin.dat" For Input As #1
    For x = 1 To 10
        Input #1, coin$(x), price(x)
    Next x
    Close #1
    limit = 10
    switch$ = "on"
    Do While switch$ = "on"
        switch$ = "off"
        For y = 1 To (limit - 1)
            If coin$(y) > coin$(y + 1) Then
                Swap coin$(y), coin$(y + 1)
                Swap price(y), price(y + 1)
                switch$ = "on"
                last = y
            End If
        Next y
        limit = last
    Loop
    Close #1
    prints ()
End Sub
Sub prints
    Print
    Print h1$
    Print h2$
    For i = 1 To 10
        Print Using p1$; coin$(i); price(i)
    Next i
    wrapup ()
End Sub
Sub wrapup
    Print "process completed"
    End
End Sub

[Moneo]

SJ Zero,

In the menu subroutine, now a sub, you converted his original gosub menu to menu (). This causes a recursive situation which is not clear how the program gets out of.

I had previously suggested replacing the gosub menu with a goto menu. You might want to do the same.
*****

[Moneo]

Axiom,

You're not paying attention to my suggestions. If you don't agree with them, say so, and we'll discuss them.

Here's my findings on the last posted version of your program:

1) At the top of the program, why do you gosub menu and then gosub shellsort. The menu lets the user decide whether to go to shellsort or not.

2) The menu subroutine still has a gosub menu when invalid. This causes recursion which you don't want. Just do a goto menu.

3) The menu subroutine will never return because it gosubs to other subroutines and has no RETURN at the end. After the END SELECT it will fall through into the process subroutine.

4) The process subroutine will never return. It has no RETURN. It either goes to wrapup or it recursively calls itself with a gosub process.

5) The shellsort subroutine will never return. It has no RETURN. At the end it does a gosub prints (which never return).

6) In general: Only set up subroutines if you intend to do a gosub to them and return from them. setup is the only real subroutine that yoo have. wrapup does not need to be a subroutine because it never wants to return.

7) Everything that I've commented on has to do with program structure and flow. If we get passed this stuff, then we can see the details of the code --- if you want.
*****