[Tutor] Re: working code for adding line numbers -- request for criticism

[Andrei]

Brian van den Broek wrote on Sat, 17 Apr 2004 15:56:43 -0400:

> I've found that traceback errors identified by line numbers can be tricky to 
> track down without line number indications in the code or the editor. (IDLE 
> doesn't seem to provide for line numbers, and it's no fun counting line on 
> screen in a long file.) So, I wrote a script to take a file and output a file 

Tip: I don't use/know a lot about Idle, but other editors (like SciTE) show
the numbers AND allow you double-click on the line number in the traceback
to jump directly to the offending line.

<snip>
> I also understand that adding comments to arbitrary lines may violate syntax 
> (say, by adding a comment to other than the last line of a long string). It also 
> may well take lines past 80 characters. Neither worry me; the line number 
> comments are being added to a new file intended for inspection rather than 
> running. Also, it is likely OS specific to DOS/Windows. That's what I run (for 
> now) and what I can test on.

If you code this kind of stuff properly, it will generally be
cross-platform without you even thinking about it :).


<snip>
> def get_filename():
>      f = raw_input('What file would you like to process?  \n \
>      (Specify a full path if it is not in the current working directory) \n')
>      # This makes for ugly screen output. How can I left allign the output
>      # without breaking the function def's indetation?

Tip 2: have a look at the discusion started by Adam, with the title "Why is
this write defined as tuple...", it contains some useful tips about
building strings that you should find helpful. More the the point: you
could either A) use print to write the string in pieces or B) make a list
of string pieces and join them like this:

       message = ["What file blabla",
                  "More text here (will be on a new line)",
                  "Third line"]
       f = raw_input("\n".join(message))

In your case, I'd opt for printing the message in pieces. Triple-quoting
the string is also an option, but that breaks the indentation visually,
which I think would be a pity in this case.

<snip>
>      try:
>          m = int(m)
>          if m > 0: return m
>          else:
>              print o; return default

I don't like the way you wrote this. It's not customary to have more than
one statement on a single line. In fact, at first I didn't even notice
there was a return in the else, I just saw "print" and thought that o and a
bunch of other stuff will be printed. I'd also recommend being consistent
in the way you indent code, so IMO the best thing is to always have a
newline + indentation after a ":".

  if m > 0:
      return m
  else:
      print o
      return default

>      except:
>          print o; return default

Same here with the ";". New lines are cheap, so use them :).

> sfilename = get_filename()
> # 'sfilename', etc. for source file name, etc.
> # Likewise for 'tfilename' and taget file name.
> print
> while os.path.isfile(sfilename) == False:
>      print "I'm sorry, but you have specified a non-existent file path.\
>      Please try again. (You can press CTRL-C to end if you prefer)"
>      sfilename = get_filename()

You should initialize loop variables immediately above the loop (for
readability). In this case you have the initialization, then some
distraction in the form of comments and a print and only after that the
loop starts.

> sfile = file(sfilename)
> sfile_base = os.path.basename(sfilename)
> i = sfile_base.rfind('.')
> tfile_base = sfile_base[:i] + '_numbered' + sfile_base[i:]

There's a battery for this included in Python: the os.path.splitext()
function. It should also solve the problem you mentioned with
extension-less filenames.

> if os.path.dirname(tfilename) == '': dirname = os.getcwd()
> # To ensure that the directory is displayed in print below. Needed as
> # os.path.dirname(tfilename) == '' when the dirname is the cwd
> else: dirname = os.path.dirname(tfilename).capitalize()

That's ugly :). I was surprized when I came across the else, already having
forgotten about the if, given the fact that the indentation seemed to jump
back to 0. Better:

  if os.path.dirname(tfilename) == '': # no directory specified
      dirname = os.getcwd() # file in current directory
  else:
      dirname = os.path.dirname(tfilename).capitalize() # DOS-style

Why don't you use the DOS-style when getcwd() is used?

Neat trick (uses the knowledge that Python regards empty
strings/lists/tuples as "False" in a boolean context and that "" or "b"
returns "b" while "a" or "b" returns "a"):

  dirname = os.path.dirname(tfilename) or os.getcwd()
  dirname = dirname.capitalize()

> tfile = file(tfilename, 'w')
> i = 1
> for line in sfile:
>      if i%multiple == 0 and line[-1] == '\n':
>          line = line[:-1] + '  # ' + str(i) + '\n'
>      if i%multiple == 0 and line[-1] != '\n':
>          line = line + '  # ' + str(i)
>      tfile.write(line)
>      i = i + 1

I'd dump the line[-1] condition and use rstrip() to get rid of any chars at
the end. By the way, I'd also put line numbers at the beginning of the line
instead of at the end. Format strings would help readability here.

-- 
Yours,

Andrei

=====
Real contact info (decode with rot13):
cebwrpg5 at jnanqbb.ay. Fcnz-serr! Cyrnfr qb abg hfr va choyvp cbfgf. V ernq
gur yvfg, fb gurer'f ab arrq gb PP.