-1
\$\begingroup\$

I created a program for drawing graphs of functions and want to hear ideas on "how to improve this program", because I'm only starting to write programs in Python. Would be very good to hear your opinion about this program. Sorry for my English, if I wrote something incorrectly.

Source code: graph in console

import math
import mpmath
import os

class Graphic:
    def __init__(self, x, y, points):
        self.x = x
        self.y = y
        self.points = points
        self.expr = ""
        self.graphic_arr = []
        self.pow = lambda x, degree: math.pow(x, degree)
        self.root = lambda x, root_of: x ** (1 / root_of)
        self.factorial = lambda x: math.factorial(x)
        self.log = lambda x, base: math.log(x, base)
        self.exp = lambda x: math.e ** x
        self.tan = lambda x: math.tan(math.degrees(x))
        self.sin = lambda x: math.sin(math.degrees(x))
        self.cos = lambda x: math.cos(math.degrees(x))
        self.cot = lambda x: mpmath.cot(math.degrees(x))
        for row in range(self.y):
            for col in range(self.x):
                self.graphic_arr.append(" ")

    def help(self):
        print("This program can draw graphics of functions what were given.")
        print("Rules of assignment functions: ")
        print("1. Never use another names of variables except 'x'")
        print("2. Operators what you can use: +(addition), -(subtraction),")
        print("*(multiplication), /(division), **(exponentiation),")
        print(" %(modulo), //(division without remainder)")
        print("3. Description of functions what you can use: ")
        print("pow(x, degree) - calculate degree of x;")
        print("root(x, root_of)  - calculate root of x;")
        print("log(x, base) - calculate logarithm of x with given base;")
        print("factorial(x) - calculate factorial of x;")
        print("exp(x) - calculate exponent of x;")
        print("sin(x), cos(x), tan(x), cot(x) - can calculate sinus,")
        print("cosinus, tangens, cotangens of x respectively.")
        print("What do those symbol on graphics mean?")
        print("@ - point, * - point of graphic, # - point on point of graphic")

    def append(self, el, index):
        new_str = ""
        for i in range(len(self.graphic_arr)):
            if i == index:
                new_str += str(el)
            else:
                new_str += self.graphic_arr[i]
        return new_str

    def fill_data_arr(self):
        data_arr_y = []
        data_arr_x = []
        result = ""
        print("You can enter 'help' if you dont know how to use this program")
        self.expr = input("f(x) = ")

        for x in range(1, self.x):
            eval_variables = {'x': x, 'root': self.root, 'pow': self.pow, 'factorial': self.factorial, 'log': self.log, 'exp': self.exp, 'sin': self.sin, 'cos': self.cos, 'tan': self.tan, 'cot': self.cot}
            if self.expr == "help":
                self.help()
                self.expr = input("f(x) = ")
            try:
                result = int(eval(self.expr, eval_variables))
            except (NameError, SyntaxError) as e:
                print("What you enter is not expression or don't exist among predefined functions!\nSource: ", e)
                os.abort()
            except ZeroDivisionError as e:
                print("You committed division by zero!\nSource", e)
                os.abort()

            data_arr_y.append(result)

            data_arr_x.append(x)

        return data_arr_x, data_arr_y

    def insert_gaps_y(self, num, full_width):
        gap = " "
        gaps = (full_width-len(num))*gap
        return gaps

    def insert_gaps_x(self):
        gap = " "
        line = " "
        for i in range(self.x):
            i += 1
            gaps = (3 - len(str(i))) * gap
            line += str(i)+gaps

        return line

    def create_points_index_arr(self):
        points_arr_x = []
        points_arr_y = []
        points_index_arr = []
        for i in range(len(self.points)):
            el = self.points[i]
            points_arr_x.append(el[0])
            points_arr_y.append(el[1]+1)
        # invert y coordinate
        for j in range(len(points_arr_y)):
            points_arr_y[j] = self.y - points_arr_y[j] + 1
        # coordinates x and y to index
        for j in range(len(points_arr_y)):
            point_index = (self.y * points_arr_y[j] - 1) - (self.y - points_arr_x[j])
            points_index_arr.append(point_index)
        return points_index_arr

    def create_index_arr(self, arr_x, arr_y):
        index_arr = []
        # invert y coordinate
        for j in range(len(arr_y)):
             arr_y[j] = len(arr_y) - arr_y[j] + 1
        # coordinates x and y to index
        for j in range(len(arr_y)):
            index = (self.y * arr_y[j]-1) - (self.y - arr_x[j])
            index_arr.append(index)

        return index_arr

    def print(self, data_arr_x, data_arr_y):
        points_index_arr = self.create_points_index_arr()
        index_arr = self.create_index_arr(data_arr_x, data_arr_y)

        for i in range(len(points_index_arr)):
            self.graphic_arr = self.append("@", int(points_index_arr[i]))

        for i in range(len(index_arr)):
            self.graphic_arr = self.append("*", int(index_arr[i]))

        for j in range(len(points_index_arr)):
            for i in range(len(self.graphic_arr)):
                if(self.graphic_arr[i] == "*" and points_index_arr[j] == i):
                    self.graphic_arr = self.append("#", i)



        index = 0
        print("     ", '^')
        print("     ", 'Y')
        for row in range(self.y-1):
            print(self.y-row-1, self.insert_gaps_y(str(self.y-row-1), 4), "|> ", end="")
            for col in range(self.x):
                print(self.graphic_arr[index]+"  ", end="")
                index += 1
            print()
        print("      0", "_|_"*self.x, 'X >')
        print("       ", self.insert_gaps_x())
        print("f(x) =", self.expr)

points = [[]]
x_or_y = "x"
element = 0
index = 0

while True:

    print("Enter", x_or_y, " coordinate element ", element+1, " or 'x' for exit")

    enter = input()

    if (enter == "x"):
        points.pop()
        os.system('cls')
        break
    if (x_or_y == "x"):
        points[element].append(int(enter))
        x_or_y = "y"
    elif (x_or_y == "y"):
        points[element].append(int(enter))
        x_or_y = "x"
        element += 1
        points.append([])

    print(points)


graphic = Graphic(10, 10, points)

data_arr_x, data_arr_y = graphic.fill_data_arr()
graphic.print(data_arr_x, data_arr_y)```
\$\endgroup\$
7
  • \$\begingroup\$ In help, a single print() would suffice, if you triple-quote: print("""This program ... of graphic"""). In particular, such a string supports embedded newlines, making the source code easier to read. We usually frown on re-defining builtins, such as list or dir. So def print seems like a mistake. Simply choose another name, print1 or something more descriptive. \$\endgroup\$
    – J_H
    Commented Mar 18, 2023 at 18:41
  • \$\begingroup\$ Thank you, I'll do it \$\endgroup\$ Commented Mar 18, 2023 at 18:43
  • \$\begingroup\$ Oh, and help is a builtin, too. You might try e.g. >>> help(print). But redefining it is less serious. Still, consider renaming the function. Or put it in a class so we call self.help(), which would be just fine. \$\endgroup\$
    – J_H
    Commented Mar 18, 2023 at 18:48
  • \$\begingroup\$ yes, add "help" function in class Graphic before posting the code on site. Have problems with formating code on StackExchange \$\endgroup\$ Commented Mar 18, 2023 at 18:51
  • 1
    \$\begingroup\$ I tried running the code. It only works if f(x) = x. Another input e.g. f(x) = sin(x) causes it to crash. If you fix the code. I can review it. Also, having to input coordinates at the start is irritating. Can u add a default value that one can use directly? \$\endgroup\$ Commented Mar 19, 2023 at 3:23

1 Answer 1

1
\$\begingroup\$

Ok, there are a few things to unpack here, so I'm not going to do a full refactored code summary at the end, but here are some things to be aware of.

PEP-8

PEP-8 is the standard style recommendation for Python. In it, it includes things like how many blank lines, how much indentation, recommendations for variable naming and casing.

There are linters (Flake8, Pylint and others) which check your code against PEP-8 and issue useful warnings. In your case there are several simple and obvious things.

This tells me immediately, for example, that line 115 is badly indented. Try spotting that by eye.

It also warns of a few unused variables (row l. 21, col, l. 22, col l. 145). These are all unused loop indices, when we don't want to use a loop index, we can use _ to say that this won't be used.

Functions as first-class objects

Since functions in Python are first class objects, that means we don't need to wrap our functions in lambdas, we can just assign them as-is.

We can also assign functions at a class level, rather than in the init if they are to be used globally and they work just like methods defined on the class.

We should also avoid using lambdas where they aren't instantaneously used and discarded. We should try to declare full functions (my linters throw warnings if you assign a lambda to a variable).

We might end up with something more like

class Graphics:
    def __init__(...):
        ...

    pow = math.pow
    factorial = math.factorial
    log = math.log
    exp = math.exp

    @staticmethod
    def root(x, root_of):
        return x ** (1 / root_of)
    ...

(@staticmethod means a method on a class which is independent of the class and provides a convenience function to that class)

Since these aren't being called from outside the class we should also prefix them with _ (e.g. _pow) which indicates "this is private" in Python.

Though since you call these mostly from the dictionary in fill_data_arr we could even avoid most of this and put the math.etc functions in there directly.

Docstrings

As J_H pointed out in the comments we can clean up that help function with the aid of a multiline string. Better yet, though we can use Python's docstrings to make this the help for the class by default. Docstrings are strings that sit at the start of a function and are what are printed when we call help(thing), and are displayed directly in some IDEs (code editors).

class Graphic:
    """
    This class can draw graphs of functions that were given.
    Rules of assignment functions:
    1. Never use another names of variables except 'x'

    2. Operators that you can use:
    +(addition);
    -(subtraction);
    *(multiplication);
    /(division);
    **(exponentiation);
    %(modulo);
    //(division without remainder)

    3. Description of functions that you can use:
    pow(x, degree) - calculate degree of x;
    root(x, root_of)  - calculate root of x;
    log(x, base) - calculate logarithm of x with given base;
    factorial(x) - calculate factorial of x;
    exp(x) - calculate exponent of x;
    sin(x), cos(x), tan(x), cot(x) - can calculate sinus,
    cosinus, tangens, cotangens of x respectively.

    What do those symbol on graphics mean?
    @ - point
    * - point of graphic
    # - point on point of graphic
    """
    ...

Then, if we call help(Graphic) or Graphic.__doc__ it will give us the appropriate help.

All functions and methods should have docstrings describing their function, usage, arguments, likely errors, etc. This means that when someone comes along to read your code, they know what everything is supposed to be doing.

We should also use type-hints, which tell the user (and some IDEs) what types the user should expect to pass into a function, what the function returns (and sometimes what type a variable should be, though this is less common to do in full). The syntax of a type hint is:

from typing import Any

def append(self, el: Any, index: int) -> str:

which says el can be any type, index should be an int and the function will return a string.

Pythonic loops

In Python loops over lists or strings, etc. generally loop over the elements of the iterable automagically. This means that something like:

for i in range(len(self.points)):
    el = self.points[i]
    points_arr_x.append(el[0])
    points_arr_y.append(el[1]+1)

could more easily be written as

for el in self.points:
    points_arr_x.append(el[0])
    points_arr_y.append(el[1]+1)

Or if points is already an array of pairs we can unpack them implicitly.

for x, y in self.points:
    points_arr_x.append(x)
    points_arr_y.append(y+1)

Functionality

graphic_arr

At the end of your init, you have the loop:

for row in range(self.y):
    for col in range(self.x):
        self.graphic_arr.append(" ")

If this is just setting graphic_arr to [" ", " ", ...] we can use a list-comprehension

self.graphic_arr = [" " for _ in range(self.x * self.y)]

Though note that your graphic_arr is later transformed into a str, so we could just do:

self.graphic_arr = " "*self.x*self.y

append

Your append function is more of an insert function as it puts el at index not the end. It could also be done without looping through graphic_arr

N.B. Because graphic_arr starts as a list, but becomes a str (as a result of this function) this approach wouldn't work, this assumes graphic_arr is a str throughout. by:

def insert(self, el: Any, index: int) -> str:
    new_str = self.graphic_arr[:index] + str(el) + self.graphic_arr[index+1:]
    return new_str

insert_gaps_

Your two functions insert_gaps_x and insert_gaps_y might want to look into using the str methods ljust, rjust and center to do the spacing.

fill_data_arr

In fill_data_arr you construct a (near) constant dictionary every cycle of the loop. This could be moved outside the loop and just update x within the loop.

Also note that if you type help twice as your expr, you will break the function. Since expr is not ordinarily changed in the loop, move the if statement outside and change it to a while to keep asking what function is while the user keeps writing "help". This also cleans up your loop.

Let error handlers handle errors in the appropriate way, don't call os.abort(), if I wrapped your function in a try-except block, I should be able to appropriately reassess my code. You can either raise your own exception with your message, or re-raise the original. This also gives helpful traceback. I would also say that receiving a ZeroDivisionError tells me quite clearly that I tried to divide by zero, and should probably not be excepted at all.

Main guard

Finally, you should insert your code outside the function into a main guard, which looks like:

if __name__ == "__main__":
   my code...

What this does is mean that if someone imports your code into their module to use the Graphic class, Python will not run my code. Any top-level code is executed in Python upon import and the main guard ensures that only those bits you want to run are run.

\$\endgroup\$
1
  • \$\begingroup\$ DeathIncarnate, thank you for this useful rewiew;) \$\endgroup\$ Commented Mar 19, 2023 at 13:23

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.