2
\$\begingroup\$

Hello everyone

In today's assignment I had to write a function to determine if (and how many) the quadratic function defined by the formula f(x) = ax^2 + bx +c has roots. I had decorators to write.

Here is my solution:

from typing import List, Tuple
from math import sqrt
from datetime import datetime

class DeltaError(Exception):
    '''Error when calculating roots, and delta is lower than 0.'''
    def __init__(self):
        Exception.__init__(self, "Delta must be greater than 0!")

class Quadratic:
    
    def __init__(self, a: int, b: int, c: int):
        self.a = a
        self.b = b
        self.c = c
        self.x = -1

       
    @property
    def Roots(self):
        a, b, c = self.a, self.b, self.c
        d = sqrt(b * b - 4 * a * c)
        if d > 0:
            x1 = (-b + d) / (2 * a)
            x2 = (-b - d) / (2 * a)
            return x1, x2
        if d == 0:
            return -b / 2 * a
        if d < 0:
            raise ZeroError
        
    @property
    def Vietas_formula(self):
        a, b, c = self.a, self.b, self.c
        d = b * b - 4 * a * c
        if d > 0:
            return "x1 + x2 = {x}".format(x = -b / a), "x1 * x2 = {y}".format(y = c / a)

    @property
    def Vertex(self):
        a, b, c = self.a, self.b, self.c
        d = b * b - 4 * a * c
        return "W: ({p}, {q})".format(p = -b / 2 * a, q = -d / 4 * a)

    @property
    def Time_dependent(self):
        if datetime.now().hour + 2 in range(8, 16):
            return "Roots"
        else: return "Break"

    @property
    def x_plus_3(self):
        a, b, c, x = self.a, self.b, self.c, self.x
        x += 3
        return x * (x * a + b) + c

    



quad = Quadratic(1, 5, 6)
print(quad.Roots, quad.Vietas_formula, quad.Vertex, quad.Time_dependent, quad.x_plus_3)

I'm counting on advice, and a better use of classes, static assignment to a, b, c and delta values, so that through inheritance I can use in subsequent functions without having to declare them from scratch.

Have a nice day!

\$\endgroup\$
4
  • 1
    \$\begingroup\$ [you] had decorations to write as in the assignment forces you to call into declarations like @property? \$\endgroup\$
    – Reinderien
    Commented May 27, 2021 at 14:02
  • \$\begingroup\$ ummm... probably im wrong, but i thought @property are parts of decorators, but as i said im probably wrong.... How should I use decorators there? \$\endgroup\$ Commented May 28, 2021 at 6:31
  • \$\begingroup\$ No you're right - @property is a decorator; your wording just befuddled me a little. \$\endgroup\$
    – Reinderien
    Commented May 28, 2021 at 14:00
  • \$\begingroup\$ And actually your use of @property overall is quite good, all things considered. \$\endgroup\$
    – Reinderien
    Commented May 28, 2021 at 14:01

1 Answer 1

3
\$\begingroup\$

Some minor stuff -

  • your member functions and properties should be lower-case by PEP8
  • you should add a property to get the discriminant, particularly since you use it in multiple places
  • I don't know what time_dependent and x_plus_3 do, nor why they're here. They seem like specific applications of the quadratic formula for a narrow situation. Given that, they should not be in this class.
  • The returns from vietas_formula should not be strings; you should return a tuple of floats. Similar for vertex. Currently this is premature stringizing. Your roots already does this correctly.
  • If you wanted to be thorough, d < 0 should not raise a ZeroError and instead should return complex roots. Python has built-in support for complex numbers.
  • self.x does not belong as a member on the class.
  • Have you renamed DeltaError to ZeroError?
  • Exception.__init__ should be changed to use super()
  • The error message Delta must be greater than 0! is not strictly true, and should be greater than or equal to zero.
  • Why must a, b, c be int? You should accept float.

Some major stuff: have you checked this for correctness? I see at least three algebraic errors, including that d = sqrt(b * b - 4 * a * c) is calling sqrt too early to catch failures, and -b / 2 * a has incorrect order of operations. Unit testing for this kind of code is both easy and important; while translating your code I ran into test failures and had to make corrections almost every step of the way.

Suggested

I still don't understand why your code needs to take a break (it's actually pretty funny. Maybe it's unionized?), but so be it:

from cmath import sqrt, isclose
from datetime import datetime, time
from numbers import Complex
from typing import Union, Tuple

RootTypes = Union[
    Tuple[Complex],
    Tuple[Complex, Complex],
]

WORK_HOURS_START = time(8)
WORK_HOURS_END = time(16)


class Quadratic:
    def __init__(self, a: float, b: float, c: float):
        self.a, self.b, self.c = a, b, c

    def y(self, x: Complex) -> Complex:
        a, b, c = self.a, self.b, self.c
        return a*x*x + b*x + c

    def dydx(self, x: Complex) -> Complex:
        a, b = self.a, self.b
        return 2*a*x + b

    @property
    def discriminant(self) -> float:
        a, b, c = self.a, self.b, self.c
        return b*b - 4*a*c

    @property
    def roots(self) -> RootTypes:
        a, b, c, d = self.a, self.b, self.c, self.discriminant
        if d == 0:
            return -b/2/a,

        sqrt_d = sqrt(d)
        return (-b + sqrt_d)/2/a, (-b - sqrt_d)/2/a

    @property
    def vietas_formula(self) -> Tuple[
        float,  # sum
        float,  # product
    ]:
        a, b, c = self.a, self.b, self.c
        return -b/a, c/a

    @property
    def vertex(self) -> Tuple[
        float,  # p
        float,  # q
    ]:
        a, b, c, d = self.a, self.b, self.c, self.discriminant
        return -b/a/2, -d/a/4

    def describe(self) -> str:
        v1, v2 = self.vietas_formula
        return (
            f'Roots: {self.roots}\n'
            f'Vieta constants: x1+x2={v1}, x1*x2={v2}\n'
            f'Vertex: {self.vertex}'
        )


def time_problem() -> None:
    if WORK_HOURS_START <= datetime.now().time() < WORK_HOURS_END:
        a, b, c = 1, 5, 6
        quad = Quadratic(a, b, c)
        print(quad.describe())
    else:
        print("I'm on a break for some reason.")


def abs_close(x: Complex, y: Complex) -> bool:
    return isclose(x, y, abs_tol=1e-12)


def test_two_real() -> None:
    q = Quadratic(1, 5, 6)

    x1, x2 = q.roots
    assert abs_close(0, x1.imag)
    assert abs_close(0, x2.imag)
    assert abs_close(0, q.y(x1))
    assert abs_close(0, q.y(x2))

    v1, v2 = q.vietas_formula
    assert abs_close(v1, x1 + x2)
    assert abs_close(v2, x1 * x2)

    vx, vy = q.vertex
    assert abs_close(0, q.dydx(vx))
    assert abs_close(vy, q.y(vx))


def test_one_real() -> None:
    q = Quadratic(9, -6, 1)

    x, = q.roots
    assert abs_close(0, x.imag)
    assert abs_close(0, q.y(x))

    # In this case Vieta's formula can be interpreted as two identical superimposed roots
    x1, x2 = x, x
    v1, v2 = q.vietas_formula
    assert abs_close(v1, x1 + x2)
    assert abs_close(v2, x1 * x2)

    vx, vy = q.vertex
    assert abs_close(0, q.dydx(vx))
    assert abs_close(0, vy)


def test_two_complex() -> None:
    q = Quadratic(4, -4, 3)

    x1, x2 = q.roots
    assert not abs_close(0, x1.imag)
    assert not abs_close(0, x2.imag)
    assert abs_close(0, q.y(x1))
    assert abs_close(0, q.y(x2))

    v1, v2 = q.vietas_formula
    assert abs_close(v1, x1 + x2)
    assert abs_close(v2, x1 * x2)

    vx, vy = q.vertex
    assert abs_close(0, q.dydx(vx))
    assert abs_close(vy, q.y(vx))


def test() -> None:
    test_two_real()
    test_one_real()
    test_two_complex()


if __name__ == '__main__':
    test()
    time_problem()
\$\endgroup\$
2
  • \$\begingroup\$ time dependence is quite simple, when the current time is between 8am and 3pm it should return roots, otherwise it should return "im having a break" \$\endgroup\$ Commented May 28, 2021 at 6:33
  • \$\begingroup\$ That's weird, but sure. There's a better way to go about comparing a time range. \$\endgroup\$
    – Reinderien
    Commented May 28, 2021 at 13:56

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.