Board index » delphi » urgent - help me with my bad code!! please

Kell

Delphi Developer
Wed, 18 Jun 1902 08:00:00 GMT

urgent - help me with my bad code!! please

I've got a program here that doesn't quite work and I have some ideas why.
What I am trying to do is get the program to read in 3 things.  The first is
a string/word that will be inserted into a linked list.  The second is a
substring (char or string) to search for a word that is to be deleted from
the list, and the third is a string that is to replace the deleted word.

As you can see, I'm bad with strings because I've only really worked with
integers.  I could really use the help with my code, let me know what I'm
doing wrong and how I could fix it, I've spent days on this already!

Also, I need to write a procedure that reads in the strings and I'm having
problems with that as well (should be simple I know)

Thanks a lot.

Here's the code so far :

program linked;

uses windos, crt;

  type
    nodeptr = ^nodetype;
    nodetype = record
      thestring: string;
      nextnode: nodeptr;
    end;

  var
    list: nodeptr;
    word: string;
    answer: char;

  procedure newlist(var list: nodeptr);
    var
      p: nodeptr;
      i: string;

    begin
      writeln('Do you want to continue? ');
      read(answer);

      new(list);
      list^.thestring := word;
      p := list;
      while answer <> 'q' do
        begin
          new(p^.nextnode);
          p^.nextnode^.thestring := i;
          p := p^.nextnode;
        end;
      p^.nextnode := nil;
    end; {procedure newlist}

  procedure writelist(list: nodeptr);
    var
      p: nodeptr;
    begin
      p := list;
      while p <> nil do
       begin
          write(p^.thestring);
          p := p^.nextnode;
        end;
    end;

  procedure insert(var list: nodeptr)
    var
      p: nodeptr;
    begin
      new(p);
      p^.thestring := word;
      p^.nextnode := list;
      if p^.nextnode = nil then
        p^.nextnode^.nextnode := nil;
      list := p;
    end;

  procedure delete(var list: nodeptr);
    var
      p, t: nodeptr;
    begin
      p := list;
      t := p^.nextnode^.nextnode;
      dispose(p^.nextnode);
      p^.nextnode := t;
    end;

  begin

    clrscr;
    writeln;
    writeln;
    writeln( 'Enter a word to be inserted ');
    read(word);
    newlist(list);
    writelist(list);

  end.

 
The Scarlet Manu

Delphi Developer
Wed, 18 Jun 1902 08:00:00 GMT

Re:urgent - help me with my bad code!! please


In comp.lang.pascal.borland, Kelli <u...@idirect.ca> wrote:

Quote
>   procedure newlist(var list: nodeptr);
>     var
>       p: nodeptr;
>       i: string;
>     begin
>       writeln('Do you want to continue? ');
>       read(answer);
>       new(list);
>       list^.thestring := word;
>       p := list;
>       while answer <> 'q' do
>         begin
>           new(p^.nextnode);
>           p^.nextnode^.thestring := i;
>           p := p^.nextnode;
>         end;
>       p^.nextnode := nil;
>     end; {procedure newlist}

Okay, there are a couple of problems here:
The writeln() and read() statements should be later, just before the while
loop in fact. This gives you a clearer picture of what is going on.
But the main problem is that the value of <answer> doesn't change inside the
loop, so unless you answer 'q' the first time, you'll get an infinite loop
(well, until you run out of heap space). So put an extra copy of the
writeln() and read() statements inside your loop (at the end of your loop,
as it is now). To make things more efficient, you can convert the loop into
a repeat..until loop; but that is up to you.

Quote
>   procedure insert(var list: nodeptr)
>     var
>       p: nodeptr;
>     begin
>       new(p);
>       p^.thestring := word;
>       p^.nextnode := list;
>       if p^.nextnode = nil then
>         p^.nextnode^.nextnode := nil;

I'm not sure why you have this if statement in here, but it's not a good
thing to have! If p^.nextnode is nil, then p^.nextnode^.nextnode is an
*undefined address*, and setting it to nil is a Bad Idea (tm). In fact you
don't need the statement at all, since you're just adding something to the
head of the list. As long as <list> has been properly initialised to nil,
you don't have to worry about its value; simply setting up p and making
p^.nextnode equal to list is sufficient (and of course the new list is then
located at p, as you have done it).

Quote
>       list := p;
>     end;

--
______________________________________________________________________
     The Scarlet Manuka,      |        Nitpickers' Party motto:
  Pratchett Quoter At Large,  |  "He who guards his lips guards his
 First Prophet of Bonni, is:  |  soul, but he who speaks rashly will
   sa...@maths.uwa.edu.au     |    come to ruin." -- Proverbs 13:3
______________________________|_______________________________________
Wolf Behrenhof

Delphi Developer
Wed, 18 Jun 1902 08:00:00 GMT

Re:urgent - help me with my bad code!! please


Quote
Kelli wrote:
> program linked;

> uses windos, crt;

You don't need these units!

Quote
>   type
>     nodeptr = ^nodetype;
>     nodetype = record
>       thestring: string;
>       nextnode: nodeptr;
>     end;

>   var
>     list: nodeptr;
>     word: string;
>     answer: char;

>   procedure newlist(var list: nodeptr);
>     var
>       p: nodeptr;
>       i: string;

>     begin
>       writeln('Do you want to continue? ');
>       read(answer);

I don't recommend using 'read' - the second call to it will probably do
something you don't expect.
Use either 'ReadLn' or 'ReadKey' (ReadKey is in the unit 'Crt')

Quote
>       new(list);
>       list^.thestring := word;
>       p := list;
>       while answer <> 'q' do
>         begin
>           new(p^.nextnode);
>           p^.nextnode^.thestring := i;
>           p := p^.nextnode;
>         end;

This loop never ends because 'answer' is left unchanged.

- Show quoted text -

Quote
>       p^.nextnode := nil;
>     end; {procedure newlist}

>   procedure writelist(list: nodeptr);
>     var
>       p: nodeptr;
>     begin
>       p := list;
>       while p <> nil do
>        begin
>           write(p^.thestring);
>           p := p^.nextnode;
>         end;
>     end;

>   procedure insert(var list: nodeptr)
>     var
>       p: nodeptr;
>     begin
>       new(p);
>       p^.thestring := word;

It is better style to make 'word' a parameter of the procedure - don't
use global variables as a replacement for parameters.

Quote
>       p^.nextnode := list;
>       if p^.nextnode = nil then
>         p^.nextnode^.nextnode := nil;

The two lines above should be deleted because they you can't assign a
value to p^.nextnode^.nextnode if p^.nextnode is nil.

Quote
>       list := p;
>     end;

>   procedure delete(var list: nodeptr);
>     var
>       p, t: nodeptr;
>     begin
>       p := list;
>       t := p^.nextnode^.nextnode;

The assignment to t is dangerous because either p or p^.nextnode could
be nil.

Quote
>       dispose(p^.nextnode);
>       p^.nextnode := t;
>     end;

If you want to delete the 1st node, try this:

procedure delete(var list: nodeptr);
var
  temp: nodeptr;
begin
  if list<>nil then begin { check if list is not empty }
    temp:=list;           { remember 1st node }
    list:=list^.nextnode; { make list point to next node }
    Dispose(temp)         { delete 1st node }
  end
end;

Quote

>   begin

>     clrscr;
>     writeln;
>     writeln;
>     writeln( 'Enter a word to be inserted ');
>     read(word);

again: Don't use read.

Quote
>     newlist(list);
>     writelist(list);

>   end.

Wolf

Other Threads

1. * URGENT * Please can someone help me * URGENT *

2. HELP - need tiff code BAD

3. Indy 8 + Threads + COM Server/Comp = Bad things, Help Please

4. BDE default install dBASE Level BAD BAD BAD

5. Localization with inherited forms : bad, bad, bad

6. Component needed very bad, please help?

7. Please, please help me with this code.

8. Urgent , Please help . TPowersock help

9. Help Urgent necessary - Please Fast Help

10. Can somebody please please help with this code