#!/usr/bin/python

import subprocess
import smtplib


def CheckFile():
    file = open('old_ip.txt', 'r')
    if ip != file.readline().strip():
        file.close()
        open('old_ip.txt', 'w').close()
        file = open('old_ip.txt', 'w')
        file.write(ip)
        file.close()
        return True
    return False


def SendMail():
    content = ip
    mail = smtplib.SMTP('smtp.yandex.ru', 587)
    mail.ehlo()
    mail.starttls()
    mail.login('Your Yandex mail login', 'Your Yandex mail password')
    mail.sendmail('Your Yandex mail login', 'Your Yandex mail login', content)


def GetIp():
    open('new_ip.txt', 'w').close()
    subprocess.Popen('./telnet.sh').wait()


GetIp()
ip = open('new_ip.txt', 'r').readline().strip()

if CheckFile() == True:
    SendMail()

 Public https://habr.com/ru/post/676454/

Share a link to this review

21.62% issue ratio

L25 Hardcoding interpreter

It is usually not a good practice to hardcode specific python interpreter at specific path: #!/usr/bin/python. Instead, use #!/usr/bin/env python, which will interpret $PATH and make your script more portable.

R21 Bad function name

By looking at function name, stranger should roughly understand what the function does. If he doesn't, then the name should be changed.

R14 Not using "early quit"

"Early quit" is a common pattern which reduces branching and indents in python code. Instead of writing if a: <1000 lines of code> else: <return / raise exception>, revert the condition: if not a: <return / raise exception>. This way the "else" clause is not needed, and program logic is straightforward: if something is wrong, quit or raise, otherwise go on.

L22 Function with side effects

Side effects are evil: it's very hard to track what function does under the hood, because function signature does not reflect side effects. Always try to write "pure" functions, i.e. those which receive inputs, do something internally, and return output. You should be able to run such function million times and get exactly the same result with nothing changed in the environment. However, side effects are totally OK if that's the only thing the function does.

L20 Hardcoded value

If some value is hidden somewhere in the middle of code, it will be very hard to find it later. Better solutions: 1) use environment variables 2) use constants in module's top section 3) use class variables.

L12 Redundant code / overengineering

This code is not really needed or may be simplified

If you want to just create a file, use Path.touch

Suggested change:
Path('new_ip.txt').touch()
L26 Executing code in global scope

When you have some code in module scope, the code will be executed when someone imports that module - which makes the module completely non-reusable. Maybe it's not meant to be reusable today, but you'll have to refactor if you need to import the module later. Use if __name__ == '__main__' to detect whether module was imported or used as a standalone script.

R27 Comparing with True / False

if a == True should be replaced with if a is True, or better just if a. if a == False should be replaced with if a is False, or better just if not a. But be careful with cases when a may be None or is an object which has special rules of conversion to boolean.

Suggested change:
if check_file():
    # ...

Create new review request