我在评论和变量命名方面苦苦挣扎。我的老师说我的评论需要更具解释性和明确性。他说我的变量名有时也令人困惑。我只是想知道您是否可以遍历我的代码,看看您是否能够理解注释以及变量名称是否合适。当然,也可以在需要的地方提出改进建议。

#Caesar Cipher

#Function that reads and writes data to the file, and calls the encryption and decryption methods.
#It isolates the key and choice of the user, and then calls the appropriate function.
def isolate():
    #Open input file in read mode
    fin = open('Input3.txt', 'r')
    #Open output file in write mode
    fout = open('Out1.txt', 'w')
    #Create a list that holds each line of the file as an element of the list
    container = fin.readlines()

    #For loop that goes through each line of the file (contianed in list) and isolates the choice and key of the user
    #Then it removes the key and choice from the split input list and calls the appropriate method.
    for i in container:
        #Each element of the list becomes a list (words) with all spaces and '\n' removed
        words = i.split()
        #The key of the user is always the last element of the split() list.
        key = int(words[len(words) - 1])
        #The choice of the user is always the second last element of the split() list.
        choice = words[len(words) - 2]
        #Remove the key and choice from the message
        words.remove(str(key))
        #Remove the choice of the user from the message for encryption/decryption.
        words.remove(choice)

        #Join together all the elements of the message in a string
        message = ' '.join(words)
        message = message.upper()

        #If the user wishes to encrypt, call the encrypt method. This method will return the encrypted message as a sting, which can then be written to the output file.
        if choice == 'e':
            #Write the newly encrypted message to the output file.
            fout.write(encrypt(message, key) + '\n')
        #If the user wishes to decrypt, call the decrypt method. This method returns the decrypted message as a string, which can then be written to the output file.
        else:
            fout.write(decrypt(message, key) + '\n')

    #Close the file to make sure data is written properly to the file.
    fout.close()

#Encryption method, which takes two parameters, the users message and key, and returns the newly encrypted message.
def encrypt(message, key):
    #Empty string that will contain the encrypted message.
    encrypted_message = ""
    #Alphabet string, which contains all the letters of the alphabet at their index locations.
    alpha = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ'

    #For loop that goes through each character of the users message and, if a space, adds it straight to the encrypted message or encrypts it through modular division.
    for i in message:
        #If the character is a space, add it to the final message.
        if i == ' ':
            #Concatenate the space to the final message
            encrypted_message += ' '
        #If the character is not a space, determine the final locatin out of index range, % by 26 and re-add the character at this index to the encrypted_message.
        else:
            #Determine the final location of the character (out of index range)
            location = key + alpha.index(i)
            #% this location by 26 to determine the location within the proper index.
            location %= 26
            #Finally, concatenate the letter at this location to the final message.
            encrypted_message += alpha[location]

    #When the function is called, have it return the final encrypted message.
    return encrypted_message


#Decryption method that takes two parameters, the users message and key, and returns the newly decrypted message.
def decrypt(message, key):
    #Create a string that reverses the alphabet, since we are moving backwards when we decrypt.
    reverse_alpha = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ'[::-1]
    #Empty string that will hold the decrypted message.
    decrypted_message = ""

    #For loop that runs through each element of the users message and, if space, re adds it to the message, or encrypts using % division and concatenates to final message.
    for i in message:
        #If the element is a space, do not encrypt; concatenate it to the final message.
        if i == ' ':
            #Concatenate the space to the final message.
            decrypted_message += ' '
        #If the element is not a space, encrypt the letter.
        else:
            #Determine the final location out of index range of the encrypted character, and store it in the variable 'location'
            location = key + reverse_alpha.index(i)
            #% this position by 26 to determine the index location in range of the letter.
            location %= 26
            #Finally, add the letter at this location to the decrypted message.
            decrypted_message += reverse_alpha[location]

    #Converts the decrypted message into lowercase form.
    decrypted_message = decrypted_message.lower()

    #When the function is called, have it 'give back' the decrypted message so that it can be written to the output file.
    return decrypted_message

#Call the function isolate5(), which initiates the program.
isolate()


评论

您的变量名对我来说似乎大部分都很好,并且注释看起来比应有的更明确。至于改进,您可以尝试使用文档字符串和函数注释。

我不会说此#Open输入文件处于读取模式,函数open会自我解释。也许将容器中的i in更改为word或其他内容。我个人认为评论太多了

不知道您的老师是否想用注释来解释,以证明您没有复制,但说您不必解释返回声明。 `#调用函数时,让它“回馈” ...`

@FooBarUser您觉得哪些评论是不必要的

这里有很多评论。此类代码的一个很好的参考是可读代码的艺术,它准确地涵盖了您所提出的问题(以及有关代码布局的其他有用提示)。

#1 楼

在大多数程序员的规范中,对每一行进行注释是多余的。有一种称为扫盲编程的编码风格,在很大程度上强调了注释,但是即使在扫盲编程中,注释每一行也会被皱眉头。什么都不要写。

您的许多评论都只是重复说明了自己的代码。例如,此注释只不过使程序员必须阅读的文本数量增加一倍:
通过将其重新定位到行的末尾,可以减少干扰:

#Open input file in read mode
fin = open('Input3.txt', 'r')


通常很容易看到每一行代码分别执行的操作。但是,注释在提供全局信息时可能非常有用。通过一次解释几行代码,您还可以更好地描述意图。
fin = open('Input3.txt', 'r')                   # 'r' = read-only


评论


\ $ \ begingroup \ $
文档字符串的好例子。但是,与识字编程的链接似乎颇具误导性。我从未见过有文化的程序员提倡逐行注释,更不用说注释每行了。想法是将代码作为计划进行结构化,而不是将设计和功能保持为单独的实体。
\ $ \ endgroup \ $
–扬·韦尼尔(Yann Vernier)
13年11月25日在22:42

#2 楼

您会发现有很多态度,但是您在此处显示的评论类型仅对作为语言学习者有用。当我查看已经熟悉该语言的代码时,注释太明确了。从本质上讲,您会得到重复执行每一行代码的注释,而这些注释通常会掩盖为什么这样的更重要的问题。

我已经知道def行定义了一个函数,open的参数是什么意思,我可以看到代码中的参数数量,并且我知道else块的条件与if。知道Python的切片约定,将words[-1]读作最后一个单词比涉及函数调用的东西(words[len(words) - 1])更容易。另外,函数的描述确实属于文档字符串,因此我可以在解释器中查找它们。

关于命名,几乎没有什么可抱怨的。 finfout可以进行扩展,主要是为了避免与fin单词混淆。迭代名称i是过去只处理单个字母变量的解释器的保留。如今,我们使用它来避免繁琐的键入,但这并不是只键入两次的好理由。诸如line之类的描述性名称会更好。一个bugbear是isolate,因为这根本没有描述该过程;请参见第4章。它似乎是对文件进行批处理,为每一行生成一个子例程调用。我认为迭代是独立处理行的,但是从调用中实际上不清楚是操纵什么内容或以何种方式进行操作。

我在评论中缺少的是一个示例,说明文件中输入和输出数据的外观。很难从解析代码中读取实际的列格式(看起来像message e|d key,其中key是整数,e表示加密,并且消息可能包含空格)。当您从choice中删除keywords时,我们也会遇到逻辑错误;您确实应该使用key = int(words.pop()),这样就不会混淆要删除的项目。往返于int并按值而不是按位搜索将中断某些行,例如secret e message e 003

最后的注释通过描述代码的作用而不是代码的作用来说明最大的问题。它们总是不同步,这时只会引起混乱。

使用Python的高级库(例如str.translatestr.rsplit)可以大大减少代码本身,但这与您对问题的命名和评论完全不同。

#3 楼

您的评论太多了,以至于它模糊了程序。例如,

# When the function is called, have it 'give back' the decrypted message so that it can be written to the output file.
return decrypted_message


return decrypted_message肯定是不言自明的吗?

实际上,这种“解释”几乎是欺骗性的-函数不会实际上并不知道(或需要知道)输出的去向-它会解密一条消息,它不在乎消息是要打印到屏幕上还是通过电子邮件发送还是丢弃。
您还在以错误的级别进行评论-解释每一行的工作方式,而不是其原因。例如,

#Converts the decrypted message into lowercase form.
decrypted_message = decrypted_message.lower()


很好,但是为什么消息要小写呢?

整个内容可能更简洁:

# Caesar Cipher
#     Encrypt or decrypt using a shift-by-N cipher

ORD_A = ord('a')

def caesar_cipher(msg, n):
    """
    Return an alphanumeric string with each letter incremented N characters
    """
    return ''.join(' ' if ch==' ' else chr((n + ord(ch) - ORD_A) % 26 + ORD_A) for ch in msg)

def process_file(infname, outfname):
    """
    Read a text file
    Encrypt or decrypt each line
    Write the result to a text file
    """
    with open(infname) as inf, open(outfname, 'w') as outf:
        for line in inf:
            # Each line is of the form 'text to encrypt [op] [n]'
            # where op is 'd' to decrypt or 'e' to encrypt
            # and n is the number of characters to shift by
            msg, op, n = line.rsplit(None, 2)

            # Decrypting by N is the same as encrypting by -N
            n   = int(n) * (1 if op=='e' else -1)

            # Encrypt and save the result
            msg = caesar_cipher(msg.lower(), n)
            outf.write(msg + '\n')

if __name__=="__main__":
    inf  = raw_input('Name of input file? ')
    outf = raw_input('Name of output file? ')
    process_file(inf, outf)


评论


\ $ \ begingroup \ $
我会将open()操作移到process_file()之外,以便调用者可以灵活地调用process_file(sys.stdin,sys.stdout)。
\ $ \ endgroup \ $
– 200_success
13年11月25日在23:50

\ $ \ begingroup \ $
写line.rsplit(maxsplit = 2)会更清楚。另外,我会将op传递给caesar_cipher(msg.lower(),int(n),op),如果op =='d',则将具有:n = -n。写入之前不需要分配,可以直接传递结果。我还发布了caesar_cipher的另一种实现,它可能更像Pythonic。
\ $ \ endgroup \ $
– Arekolek
16 Jul 15 '16:10



#4 楼

我了解您欢迎您提出意见以外的建议:


当然,在您认为需要的地方提出改进建议。


您的encode可以重写decodea[-i:]函数,以更好地利用Python设施:它也很适合解码,因为a[len(a)-i:]在Python中的意思是dictget具有方便的dict方法,该方法可让我们提供默认值,以在''.join()中不存在该键时返回。这样,我们无需检查字符是否为空格,如果为空格则返回空格,而是将其用作默认值。免费地,它可以处理其他字符,例如数字或标点符号。

代替循环,使用生成器表达式并将其作为参数传递给alphabet

通过将shift.get(c, c)设为参数,我们允许使用更多的字符来进行密码编码。

最后,我提供了一个文档字符串来记录该方法,以解决您注释代码的主要问题。

请注意,如果我遵循您原始帖子中代码的注释风格,则必须将所有关于代码工作方式的解释作为注释添加到此答案中。但是,这不是评论的目的。在出于非学习目的编写代码时,您假定阅读代码的人员要么非常了解该语言,要么可以查看文档中的某些特定部分。因此,我假设读者不了解dict.get的工作原理,而是评论caeser_cipher的工作原理,而不是发表评论。我留下评论说不是字母的字符保持不变,但是我没有解释它是如何发生的,因为我希望可以很容易地从代码和文档中推断出来。

注释中的解释是我们使用的语言或库未提供的内容。应当为我们创建的逻辑使用注释。


或者,str.translate可以使用Yann Vernier建议的str.maketrans,并使用q4312079q,例如: br />
哪个可以实现更好的实现。

评论


\ $ \ begingroup \ $
'abcdefghijklmnopqrstuvwxyz'可以(可能应该)写为string.ascii_lowercase。
\ $ \ endgroup \ $
– 200_success
16年7月15日在17:07

#5 楼

OP中的代码注释过度。 (没有冒犯)

其他答案已经涵盖了所有基础,很好,但是我要从Yann Vernier的答案中强调一件事:


最后的注释说明了最大的问题,这些注释描述了代码的作用,而不是代码的作用。它们总是不同步,这时只会造成混乱。


这通常是注释过多的代码的最大问题。这是一个严重的问题:


注释就像代码库中的任何其他内容一样,必须加以维护。
不同步的注释不仅不值钱;他们很危险。

就个人而言,我认为在一周中的任何一天,评论不足都会比评论过多更好。分配是一回事。完成后,它不会改变。但是,大多数代码仍然存在,并且各种人都在更改它,每个人都必须消化并维护代码和注释。

注释时请保持选择性。