Sphinx - Vlada Rapaport by vladarap88 · Pull Request #23 · Ada-C22/swap-meet

vladarap88 · 2024-10-04T04:21:49Z

No description provided.

…ave 5 but haven't completed it yet

…ssion

…ategory_reordered()

kelsey-steven-ada

Great work! I've left a mix of suggestions and questions to consider for feedback. Please reply here on Github or reach out on Slack if there's anything I can clarify =]

kelsey-steven-ada · 2024-10-09T02:03:41Z

+- Importing modulesWave 5
+In Wave 5 we will create three additional modules with three additional classes.
+
+Our new modules should be defined as follows:
+
+Clothing
+
+Has an attribute id that is by default a unique integer
+Has an attribute fabric that is by default the string "Unknown"
+This attribute describes what fabric the clothing is made from; some example values might be "Striped", "Cotton", or "Floral"
+When we instantiate an instance of Clothing, we can optionally pass in a string with the keyword argument fabric
+Has a function get_category that returns "Clothing"
+Has a stringify method that returns "An object of type Clothing with id <id value>. It is made from <fabric value> fabric."
+For example, if we had a Clothing instance with an id of 123435 and a fabric attribute that holds "Wool", its stringify method should return "An object of type Clothing with id 12345. It is made from Wool fabric."
+Decor
+
+Has an attribute id that is by default a unique integer
+Holds 2 integer attributes width and length
+Both of these values should be 0 by default
+When we instantiate an instance of Decor, we can optionally pass in integers with the keyword arguments width and length
+Has a function get_category that returns "Decor"
+Has a stringify method that returns "An object of type Decor with id <id value>. It takes up a <width value> by <length value> sized space."
+For example, if we had a Decor instance with an id of 123435, width of 3, and length of 7, its stringify method should return "An object of type Decor with id 12345. It takes up a 3 by 7 sized space."
+Electronics
+
+Has an attribute id that is by default a unique integer
+Has an attribute type that is by default the string "Unknown"
+This attribute describes what kind of electronic device this is. Some example values might be “Kitchen Appliance”, “Game Console”, or “Health Tracker”
+When we initialize an instance of Electronics, we can optionally pass in a string with the keyword argument type
+Has an function get_category that returns "Electronics"
+Has a stringify method that returns "An object of type Electronics with id <id value>. This is a <type value> device."
+For example, if we had an Electronics instance with an id of 123435 and type attribute of "Mobile Phone", its stringify method should return "An object of type Electronics with id 12345. This is a Mobile Phone device."
+All three new classes and the Item class have an attribute called condition, which can be optionally provided in the initializer. The default value should be 0
+
+All three new classes and the Item class have an instance method named condition_description, which should describe the condition in words based on the value, assuming they all range from 0 to 5.
+
+These can be basic descriptions (eg. 'mint', 'heavily used') but feel free to have fun with these (e.g. 'You probably want a glove for this one...").
+The one requirement is that all the classes share the same condition_description behavior.
+Using Inheritance
+Now, we may notice that these three classes hold the same types of state and have the same general behavior as Item. That makes this is a great opportunity to use inheritance! If you haven't already, go back and implement the Clothing, Decor, and Electronics classes so that they inherit from the Item class. This should eliminate repetition in your code and greatly reduce the total number of lines code in your program!
+
+Tip: Importing Item
+You'll need to refer to Item in order to declare it as a parent. To reference the Item class from these modules, try this import line:
+
+from swap_meet.item import Item


I don't know the exact cause of the Wave 5 instructions being added to the top of the README, but this could indicate that there is an opportunity to use git status more often or practice committing files one at a time to ensure only the files we want to change are included in a PR.

If we missed that we included something we didn't mean to while developing, when we go to create a PR, GitHub will list out the changes so we can review the files included before we officially create the PR. This is a great opportunity to check our work and ensure that we only see intended changes before starting the review process.

kelsey-steven-ada · 2024-10-09T02:06:21Z

+    def __str__(self):
+        return f"An object of type {self.category} with id {self.id}. It is made from {self.fabric} fabric."
+
+    # def condition_description(self):


Love seeing that the child classes are inheriting condition_description from Item!

kelsey-steven-ada · 2024-10-09T02:08:18Z

+    # def condition_description(self):
+    #     if self.condition == 0:
+    #         return "Brand new"
+    #     elif self.condition ==1:
+    #         return "Gently used"
+    #     elif self.condition == 2:
+    #         return "Used"
+    #     elif self.condition == 3:
+    #         return "Well used"
+    #     elif self.condition == 4:
+    #         return "Heavily used"
+    #     elif self.condition == 5:
+    #         return "Heavily worn"
+
+    #     return "Unknown condition"


When we refactor, we should delete code that is no longer necessary. If we are making regular commits in git, we can use the git history to look back on previous work. In GitHub, if you go to the "Commits" tab on a repo, you can view the source code at any of the commits.

kelsey-steven-ada · 2024-10-09T02:12:18Z

+        self.condition = condition
+
+    def __str__(self):
+        return f"An object of type {self.category} with id {self.id}."


Every child class starts with a sentence of this exact form. A child class can call their parent's functions using super.

How could we share this function with the child classes to reduce similar or repeated code in their __str__ functions?

kelsey-steven-ada · 2024-10-09T02:14:26Z

+        if self.condition == 0:
+            return "Brand new"
+        elif self.condition ==1:
+            return "Gently used"
+        elif self.condition == 2:
+            return "Used"
+        elif self.condition == 3:
+            return "Well used"
+        elif self.condition == 4:
+            return "Heavily used"
+        elif self.condition == 5:
+            return "Heavily worn"


We don't test for this, and it isn't a requirement, but something to consider:

In the Wave 5 description the condition can be 0-5, so what happens if the condition is a decimal rather than an integer?

How could we update the function to handle both types?

kelsey-steven-ada · 2024-10-10T22:58:00Z

+
+        # assume the first item is the best one
+        best_item = items[0]
+        for item in items[1:]: 


There is a tradeoff happening here: by using slicing we're creating a new list in memory, making the function O(n) for space complexity to save making one extra comparison for the first item. If we want to iterate over a specific range of a list, we could use a for loop with the range function instead to keep our space complexity O(1).

kelsey-steven-ada · 2024-10-10T23:00:50Z

+
+

The PEP8 convention for blank lines is to use a single blank line to separate methods inside of a class. More info here: https://peps.python.org/pep-0008/#blank-lines

Not all projects you see as examples may follow PEP8, but the most important thing is to use whitespace consistently across a project so that readers can understand what different amounts of space mean.

kelsey-steven-ada · 2024-10-10T23:20:03Z

    # *********************************************************************
    # ****** Complete Assert Portion of this test **********
    # *********************************************************************
+    assert result == False


Something to consider, if the function returns False does that guarantee the function did not alter the inventory? Checking the vendor.inventory contents as well would confirm there are no unintended side effects.

What could that look like?

kelsey-steven-ada · 2024-10-10T23:29:27Z

+    assert item_b in fatimah.inventory
+    assert nobodys_item not in jolie.inventory
+    assert jolie.inventory == []
+    assert fatimah.inventory == [item_a, item_b, item_c]


We could remove the assertion on line 138. If we check that jolie.inventory is an empty list, then we don't need to check if nobodys_item is in jolie.inventory - It can't be present if the list is empty.

Similarly, we could remove the assertion on line 137, since below we check all the contents of fatimah.inventory.

kelsey-steven-ada · 2024-10-10T23:39:59Z

+    assert item_f in tai.inventory
+    assert item_c in jesse.inventory
+    assert item_c not in tai.inventory
+    assert item_f not in jesse.inventory


What other checks could we add to confirm that there were no unintended changes to the other items in the inventories?

vladarap88 and others added 8 commits September 30, 2024 21:09

wave 1 (class Vendor func add & remove) + test

8b4ce1b

created wave 2 and 3 and added asserts to test for wave 3.. started w…

5d73c26

…ave 5 but haven't completed it yet

completed wave4 code + unit tests are passing

173e03b

Completed wave 5 and passed all tests with Vlada during Co-working se…

0c9826f

…ssion

completed wave 6

164c7e2

added asserts to test_swap_best_by_category() and test_swap_best_by_c…

b8e84da

…ategory_reordered()

2 test wave 6

ce9c2bf

Refactored code to make more DRY

68b6331

kelsey-steven-ada reviewed Oct 10, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sphinx - Vlada Rapaport#23

Sphinx - Vlada Rapaport#23
vladarap88 wants to merge 8 commits into
Ada-C22:mainfrom
vladarap88:main

vladarap88 commented Oct 4, 2024

Uh oh!

kelsey-steven-ada left a comment

Uh oh!

kelsey-steven-ada Oct 9, 2024

Uh oh!

kelsey-steven-ada Oct 9, 2024

Uh oh!

kelsey-steven-ada Oct 9, 2024

Uh oh!

kelsey-steven-ada Oct 9, 2024

Uh oh!

kelsey-steven-ada Oct 9, 2024

Uh oh!

kelsey-steven-ada Oct 10, 2024

Uh oh!

kelsey-steven-ada Oct 10, 2024

Uh oh!

kelsey-steven-ada Oct 10, 2024

Uh oh!

kelsey-steven-ada Oct 10, 2024

Uh oh!

kelsey-steven-ada Oct 10, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

vladarap88 commented Oct 4, 2024

Uh oh!

kelsey-steven-ada left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants