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

Brian van den Broek bvande at po-box.mcgill.ca
Sun Apr 18 14:00:51 EDT 2004


Hi All,

thanks to Alan and Andrei for the feedback. (And to Llyod from a previous
reply and to those who wrote me off-list, too!)

I'm consolidating responses to Andrei and Alan into one post. Replies
interspersed below. There are a couple of places where I don't quite 
understand and have asked follow-ups. But I do apologize that this is 
getting to be a l_o_n_g post. :-(


> 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 
> 


Andrei:
> 
> 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.

Alan:
> 
> FWIW IDLE has a "Go To Line" menu option (Alt+G) which takes you to
> the line you need.
> 
<SNIP>
> 
> But it's a good enough idea since notepad users (why???)
> can use it. 
> 

The IDLE tip makes the whole thing largely moot, but it was worthwhile to do
for the learning anyway. Plus, I can imagine other situations where I might
use it, so no effort wasted.

As for why Notepad -- well, Bill invented computers, right? (Just after the
wheel and fire.) So Notepad must be the best editor after all! ;-)


Andrei:
>>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 :).

I was mostly thinking about the lines:

else: dirname = os.path.dirname(tfilename).capitalize()
# Capitalize ensures that drive letter displays in DOS convention
target = (tfile_base, dirname)
print 'Your numbered file will be %s in the directory %s' %target

Since the user could type "c:\myfile.txt" without that .capitalize() method
the dirname came out as "c:". Forcing it to be "C:" seemed the part specific
to DOS/Win systems. (I don't know the Lin/UNix drive conventions, but Alan
pointed out in a previous post that something I was doing was DOS drive
convention centered.) But that is a small thing.

One factor that made me pick Python to try and learn programming was the
frequent claim of ease of cross-platform coding. I've been to two
install-fests and have yet to come away with a working Linux install on my
laptop :-( , but I hope to be free soon :-), and definitely didn't want my
early steps in programming to be Win-centric.


Alan:
>>fil_numberede. I can see how to fix that (by constructing the output
>>filename differently depending on whether the filename contains a 
>>'.' or not, but it didn't seem worth doing.
> 
> 
> Or put "numbered_" in front instead:
> 
> numbered_file.txt

That would be easier, and I thought about it. But in a directory with a lot
of files it would make the original and the processed file be far away from
each other in a directory listing. I will think about which inconvenience is
more important to me.


Andrei:
> <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.

I like the join suggestion. I'd originally tried the triple quote but didn't
like the loss of visual indent either. Since I posted, I've changed it to
triple quote assignments outside the function definition, but the join is
nice in that it lets the indent be respected and function-specific text stay
within the function. Thanks!


Andrei:
> <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 ":".

<SNIP>

>>     except:
>>         print o; return default
> 
> 
> Same here with the ";". New lines are cheap, so use them :).

I'd thought it the multiple statements on a line OK for simple statements
and a good thing in that more code could be seen on the screen at once. But
the fact that on a first read someone with much more Python experience than
I missed the else clause's return makes me think that in a few months I'd
likely miss it too. So, no more multiple statement lines for me! Thanks.


Andrei:
> 
> 
>>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.

I absolutely see the point with respect to the print statement. But I wonder
then where to put the comments on the variable names. I put them there as
that is where the "sfilename" is defined and I thought that a longer, more
self-documenting name such as "source_filename" would be getting too long.
(I don't recall if it was this variable or another, but I do know I ended up
shortening some names so as to more easily live with 80 character lines.)
So, where to document the name? Above its first use seems odd to me. Is that
the normal Python-practice? If so, I'll get over seeing it as odd :-)


Andrei:
<SNIP>
> 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?

As for the ugly, you've convinced me. :-)

But in the last line quoted above, I'm not sure what you mean. It seemed to
me that os.getcwd() did return a capitalized drive letter and the only time
I didn't have a CAPped drive was when the user input was of the from
"d:\path\file" where 'd' is an small-case valid drive letter. (IOW,
getcwd().capitalize() seemed redundant.) Have I missed your point?


Andrei:
> 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()

Much cooler than what I had! Thanks.


Andrei:
>>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.

I don't (as yet) follow the rstrip() suggestion, but I will have a look and
post again if I don't clear it up for myself.

The line numbers at the front is how I did it at first. But unless I miss
your meaning, it seemed a bad idea. Consider:

def some_function:
     first line here
     if some condition:
         third line here
     fourth line here
     fifth line here

assuming that the def line was the first line of the file (not likely, but
its an e.g. :-) ) and every 5 lines were numbered, this would get rendered
something like:

def some_function:
     first line here
     if some condition:
         third line here
# 5    fourth line here
     fifth line here

That would, I think, make it harder for me to process what belonged to what
block.

Anyway, thanks again to all who've advised. I've learned a fair bit and been
talked out of a few bad practices before they became habitual. A good result!

Best,

Brian vdB








More information about the Tutor mailing list