Message-ID: <3B8A12E7.46777FC8@falconsoft.be>
Date: Mon, 27 Aug 2001 11:29:11 +0200
From: Tim Van Holder <tim DOT vanholder AT falconsoft DOT be>
Organization: Anubex (www.anubex.com)
X-Mailer: Mozilla 4.77 [en] (X11; U; Linux 2.2.16-3 i686)
X-Accept-Language: en, nl-BE, nl
MIME-Version: 1.0
Newsgroups: comp.os.msdos.djgpp
Subject: Re: Text Encoder Program HELP
References: <TZ_h7.4880$Ee DOT 32352 AT news1 DOT rdc1 DOT nsw DOT optushome DOT com DOT au>
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Lines: 83
NNTP-Posting-Host: 194.78.64.238
X-Trace: 998904519 reader1.news.skynet.be 36475 194.78.64.238
X-Complaints-To: abuse AT skynet DOT be
To: djgpp AT delorie DOT com
DJ-Gateway: from newsgroup comp.os.msdos.djgpp
Reply-To: djgpp AT delorie DOT com

usha1 wrote:
> 
> string message; //message to encode
> char ch; //next message character
> int pos; //its position
Two important notes: put declarations where they are used,
and always initialize.  ch and pos should be declared
and initialized inside the for loop that uses them.

> getline(cin, message, 'f');
OK - encoding when Ctrl-Z is easy: then cin.eol()
will return true (but possibly only after the next
getline() - I'd have to check).  If you need to
handle f6, you can't use cin, as it doesn't handle
those keys - you'll have to use getkey() to do your
input.

> // Encode message.
> 
> for (int x = 0; x < message.length(); x++)
Please cache message.length() somewhere (preferably
in a const size_t).  You don't change message's length,
so there's no need to keep calling length().

> {
>  ch = tolower(message.at(x)); //ch to lowercase
And since you only use x as an index into message, why
aren't you a string::iterator instead?

>  pos = ALPHABET.find(ch);     //find position of ch
>  if ((pos >= 0) && (pos < 36))
Hardcoded positions are evil - that should be
  if (pos >= 0 && pos < ALPHABET.length())
Additionally, the first test is superfluous, as find()
is guaranteed to be either a valid index into the string,
or equal to the string's length).  Again, caching the
length would be useful.
ALso, I thoroughly dislike using all-caps names for
anything other than macros.

>  message[x] = CODE.at(pos);
Simply add \n to both alphabet and code and it will be
retained in the output.

>  else
>  message[x]=' ';
> }
> 
> for (int y = 0; y < message.length(); y++)
why use y? Use another iterator (heck, use the same one).
Even better, use two strings (one for input, one for
output, and you won't need this pass at all.
You could even just use cin.get() and put the encoding
into a string.  This approach is used below.

int
main(void)
{
// Note that the fact that these strings will be easily found
// in the onject/executable makes the encoding pretty useless
// (besides it being extremely simplistic to begin with)
const string SourceAlphabet = "abc123\n";
const string TargetAlphabet = "def456\n"
  assert (SourceAlphabet.length () == TargetAlphabet.length ());
const size_t AlphabetSize = SourceAlphabet.length ();
string encoded = "";
char ch = cin.get ();
  while (!cin.eof ()) {
  size_t pos = SourceAlphabet.find (tolower (ch));
    if (pos < AlphabetSize)
      encoded.append (TargetAlphabet[pos]);
    ch = cin.get ();    
  }
  cout << "Encoded Message:" << endl
       << encoded << endl
       << "<End>" << endl;
  return 0;
}

With gcc 3.0.1-cvs on Linux, there seems to be a bug where EOF
needs to be pressed twice in a row to register if not at the start
of a new line (library bug?). Other than that, it seems to work
fine.