Skip to main content
unnecessary assignement
Source Link
Caridorc
  • 28.1k
  • 7
  • 55
  • 138

Your code is overall clean and well written.


You should use list comprehension:

def find_lower_squares(num):
    """Returns a list of even perfect squares less than a given integer"""
    squares = []
    for i in range(num, 3, -1):
        if is_even_square(i): squares.append(i)
    return squares

Should become:

def find_lower_squares(num):
    """Returns a list of even perfect squares less than a given integer"""
    return [i for i in range(num, 3, -1) if is_even_square(i)]

Scientific/mathematical code like this greatly benefits from automated testing, let me show you an example:

import doctest

def find_lower_squares(num):
    """
    Returns a list of even perfect squares less than a given integer

    >>> find_lower_squares(40)
    [36, 16, 4]
    """
    return [i for i in range(num, 3, -1) if is_even_square(i)]

doctest.testmod()

This gives double benefits:

  1. Changes to the code can be made without being worried: if you break something, the error message will show up immediately
  2. Code readability and documentation increase a lot, the reader can see some examples of the function being used and will understand it faster and better.

Avoid None-important assignment

In mid level languages such as C you must declare array first as empty and then fill them. In high level languages such as Python you should avoid such low level behaviour and insert the values in the arrays directly:

def generate_square_spiral(num):
    """
    Generates a square spiral matrix from a given integer

    >>> generate_square_spiral(5)
    [[5, 4], [2, 3]]
    >>> generate_square_spiral(17)
    [[17, 16, 15, 14], [5, 4, 3, 13], [7, 1, 2, 12], [8, 9, 10, 11]]
    """
    edge = int(sqrt(num))
    square_spiral = [nth_row(num, row) for row in range(edge)]
    return square_spiral

Also we could remove the unnecessary assignment:

edge = int(sqrt(num))
return [nth_row(num, row) for row in range(edge)]

Your code is overall clean and well written.


You should use list comprehension:

def find_lower_squares(num):
    """Returns a list of even perfect squares less than a given integer"""
    squares = []
    for i in range(num, 3, -1):
        if is_even_square(i): squares.append(i)
    return squares

Should become:

def find_lower_squares(num):
    """Returns a list of even perfect squares less than a given integer"""
    return [i for i in range(num, 3, -1) if is_even_square(i)]

Scientific/mathematical code like this greatly benefits from automated testing, let me show you an example:

import doctest

def find_lower_squares(num):
    """
    Returns a list of even perfect squares less than a given integer

    >>> find_lower_squares(40)
    [36, 16, 4]
    """
    return [i for i in range(num, 3, -1) if is_even_square(i)]

doctest.testmod()

This gives double benefits:

  1. Changes to the code can be made without being worried: if you break something, the error message will show up immediately
  2. Code readability and documentation increase a lot, the reader can see some examples of the function being used and will understand it faster and better.

Avoid None-important assignment

In mid level languages such as C you must declare array first as empty and then fill them. In high level languages such as Python you should avoid such low level behaviour and insert the values in the arrays directly:

def generate_square_spiral(num):
    """
    Generates a square spiral matrix from a given integer

    >>> generate_square_spiral(5)
    [[5, 4], [2, 3]]
    >>> generate_square_spiral(17)
    [[17, 16, 15, 14], [5, 4, 3, 13], [7, 1, 2, 12], [8, 9, 10, 11]]
    """
    edge = int(sqrt(num))
    square_spiral = [nth_row(num, row) for row in range(edge)]
    return square_spiral

Your code is overall clean and well written.


You should use list comprehension:

def find_lower_squares(num):
    """Returns a list of even perfect squares less than a given integer"""
    squares = []
    for i in range(num, 3, -1):
        if is_even_square(i): squares.append(i)
    return squares

Should become:

def find_lower_squares(num):
    """Returns a list of even perfect squares less than a given integer"""
    return [i for i in range(num, 3, -1) if is_even_square(i)]

Scientific/mathematical code like this greatly benefits from automated testing, let me show you an example:

import doctest

def find_lower_squares(num):
    """
    Returns a list of even perfect squares less than a given integer

    >>> find_lower_squares(40)
    [36, 16, 4]
    """
    return [i for i in range(num, 3, -1) if is_even_square(i)]

doctest.testmod()

This gives double benefits:

  1. Changes to the code can be made without being worried: if you break something, the error message will show up immediately
  2. Code readability and documentation increase a lot, the reader can see some examples of the function being used and will understand it faster and better.

Avoid None-important assignment

In mid level languages such as C you must declare array first as empty and then fill them. In high level languages such as Python you should avoid such low level behaviour and insert the values in the arrays directly:

def generate_square_spiral(num):
    """
    Generates a square spiral matrix from a given integer

    >>> generate_square_spiral(5)
    [[5, 4], [2, 3]]
    >>> generate_square_spiral(17)
    [[17, 16, 15, 14], [5, 4, 3, 13], [7, 1, 2, 12], [8, 9, 10, 11]]
    """
    edge = int(sqrt(num))
    square_spiral = [nth_row(num, row) for row in range(edge)]
    return square_spiral

Also we could remove the unnecessary assignment:

edge = int(sqrt(num))
return [nth_row(num, row) for row in range(edge)]
High level
Source Link
Caridorc
  • 28.1k
  • 7
  • 55
  • 138

Your code is overall clean and well written.


You should use list comprehension:

def find_lower_squares(num):
    """Returns a list of even perfect squares less than a given integer"""
    squares = []
    for i in range(num, 3, -1):
        if is_even_square(i): squares.append(i)
    return squares

Should become:

def find_lower_squares(num):
    """Returns a list of even perfect squares less than a given integer"""
    return [i for i in range(num, 3, -1) if is_even_square(i)]

Scientific/mathematical code like this greatly benefits from automated testing, let me show you an example:

import doctest

def find_lower_squares(num):
    """
    Returns a list of even perfect squares less than a given integer

    >>> find_lower_squares(40)
    [36, 16, 4]
    """
    return [i for i in range(num, 3, -1) if is_even_square(i)]

doctest.testmod()

This gives double benefits:

  1. Changes to the code can be made without being worried: if you break something, the error message will show up immediately
  2. Code readability and documentation increase a lot, the reader can see some examples of the function being used and will understand it faster and better.

Avoid None-important assignment

In mid level languages such as C you must declare array first as empty and then fill them. In high level languages such as Python you should avoid such low level behaviour and insert the values in the arrays directly:

def generate_square_spiral(num):
    """
    Generates a square spiral matrix from a given integer

    >>> generate_square_spiral(5)
    [[5, 4], [2, 3]]
    >>> generate_square_spiral(17)
    [[17, 16, 15, 14], [5, 4, 3, 13], [7, 1, 2, 12], [8, 9, 10, 11]]
    """
    edge = int(sqrt(num))
    square_spiral = [nth_row(num, row) for row in range(edge)]
    return square_spiral

Your code is overall clean and well written.


You should use list comprehension:

def find_lower_squares(num):
    """Returns a list of even perfect squares less than a given integer"""
    squares = []
    for i in range(num, 3, -1):
        if is_even_square(i): squares.append(i)
    return squares

Should become:

def find_lower_squares(num):
    """Returns a list of even perfect squares less than a given integer"""
    return [i for i in range(num, 3, -1) if is_even_square(i)]

Scientific/mathematical code like this greatly benefits from automated testing, let me show you an example:

import doctest

def find_lower_squares(num):
    """
    Returns a list of even perfect squares less than a given integer

    >>> find_lower_squares(40)
    [36, 16, 4]
    """
    return [i for i in range(num, 3, -1) if is_even_square(i)]

doctest.testmod()

This gives double benefits:

  1. Changes to the code can be made without being worried: if you break something, the error message will show up immediately
  2. Code readability and documentation increase a lot, the reader can see some examples of the function being used and will understand it faster and better.

Your code is overall clean and well written.


You should use list comprehension:

def find_lower_squares(num):
    """Returns a list of even perfect squares less than a given integer"""
    squares = []
    for i in range(num, 3, -1):
        if is_even_square(i): squares.append(i)
    return squares

Should become:

def find_lower_squares(num):
    """Returns a list of even perfect squares less than a given integer"""
    return [i for i in range(num, 3, -1) if is_even_square(i)]

Scientific/mathematical code like this greatly benefits from automated testing, let me show you an example:

import doctest

def find_lower_squares(num):
    """
    Returns a list of even perfect squares less than a given integer

    >>> find_lower_squares(40)
    [36, 16, 4]
    """
    return [i for i in range(num, 3, -1) if is_even_square(i)]

doctest.testmod()

This gives double benefits:

  1. Changes to the code can be made without being worried: if you break something, the error message will show up immediately
  2. Code readability and documentation increase a lot, the reader can see some examples of the function being used and will understand it faster and better.

Avoid None-important assignment

In mid level languages such as C you must declare array first as empty and then fill them. In high level languages such as Python you should avoid such low level behaviour and insert the values in the arrays directly:

def generate_square_spiral(num):
    """
    Generates a square spiral matrix from a given integer

    >>> generate_square_spiral(5)
    [[5, 4], [2, 3]]
    >>> generate_square_spiral(17)
    [[17, 16, 15, 14], [5, 4, 3, 13], [7, 1, 2, 12], [8, 9, 10, 11]]
    """
    edge = int(sqrt(num))
    square_spiral = [nth_row(num, row) for row in range(edge)]
    return square_spiral
Source Link
Caridorc
  • 28.1k
  • 7
  • 55
  • 138

Your code is overall clean and well written.


You should use list comprehension:

def find_lower_squares(num):
    """Returns a list of even perfect squares less than a given integer"""
    squares = []
    for i in range(num, 3, -1):
        if is_even_square(i): squares.append(i)
    return squares

Should become:

def find_lower_squares(num):
    """Returns a list of even perfect squares less than a given integer"""
    return [i for i in range(num, 3, -1) if is_even_square(i)]

Scientific/mathematical code like this greatly benefits from automated testing, let me show you an example:

import doctest

def find_lower_squares(num):
    """
    Returns a list of even perfect squares less than a given integer

    >>> find_lower_squares(40)
    [36, 16, 4]
    """
    return [i for i in range(num, 3, -1) if is_even_square(i)]

doctest.testmod()

This gives double benefits:

  1. Changes to the code can be made without being worried: if you break something, the error message will show up immediately
  2. Code readability and documentation increase a lot, the reader can see some examples of the function being used and will understand it faster and better.