Skip to content

Boids: Full refactor#8

Open
bozcar wants to merge 1 commit intoUCL-MPHY0021-21-22:mainfrom
bozcar:main
Open

Boids: Full refactor#8
bozcar wants to merge 1 commit intoUCL-MPHY0021-21-22:mainfrom
bozcar:main

Conversation

@bozcar
Copy link

@bozcar bozcar commented Dec 9, 2021

Group 23

Copy link
Contributor

@ageorgou ageorgou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice, your version clearly shows what each part of the code does, through the method names and their docstrings.

Nice use of typing hints.

As some of the lines (especially when calling methods) are quite long, you could break them down, keeping one argument per line, and using some intermediate variables.
For example:

self.update_vel([(other.velocity['x']-self.velocity['x'])*scaling/number_of_boids, (other.velocity['y']-self.velocity['y'])*scaling/number_of_boids])

could become

self.updat_vel([
    (other.velocity['x']-self.velocity['x'])*scaling/number_of_boids,
    (other.velocity['y']-self.velocity['y'])*scaling/number_of_boids
])

or

x_velocity_diff = (other.velocity['x']-self.velocity['x'])*scaling/number_of_boids
y_velocity_diff = (other.velocity['y']-self.velocity['y'])*scaling/number_of_boids
self.update_vel(x_velocity_diff, y_velocity_diff)

self.velocity = {'x' : x_velocity, 'y' : y_velocity}

def update_pos(self):
self.position['x'] += self.velocity['y']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing this is a typo!

Suggested change
self.position['x'] += self.velocity['y']
self.position['x'] += self.velocity['x']


def away_from_neighbours(self:Boid, other:Boid, number_of_boids:int, distance = 100):
"""Updates the velocity of self so that it moves away from other if other is nearer than distance"""
if (other.position['x']-self.position['x'])**2 + (other.position['y']-self.position['y'])**2 < distance:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is quite long, and is repeated in the other method. Perhaps it could be extracted into its own method, such as can_see(self:Boid, other:Boid).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants