maxsu / IFProject

GNU Affero General Public License v3.0
0 stars 1 forks source link

Implement and Test of `get_address` method for `Node` Class #11

Open maxsu opened 8 months ago

maxsu commented 8 months ago

We are finalizing the details of the get_address method for the AST Node class in our game engine, specifically on the GetAddress branch. The method signature will be Node.get_address(*args), with args being a mix of strings (for Map nodes) and integers (for Sequence nodes). This discussion includes method implementation, exception handling, and test case development, excluding performance considerations at this stage.

Requirements

Exceptions

Test Cases

  1. test_get_address_valid_map_node: Ensure doc_node.get_address('doc', 'vars', 2) returns a Var node instance with the correct value in Var.data["value"].

  2. test_get_address_valid_sequence_node: Verify accessing a valid index in a sequence returns the expected node.

  3. test_get_address_invalid_key_raises_BadAddress: Confirm that accessing a non-existent key in a map node raises a BadAddress exception.

  4. test_get_address_invalid_index_raises_BadAddress: Check that accessing an out-of-range index in a sequence raises a BadAddress exception.

  5. test_get_address_type_mismatch_raises_BadNode: Validate that attempting to access a node attribute of the wrong type raises a BadNode exception.

  6. test_get_address_on_nonexistent_node_raises_BadAddress: Ensure that trying to access a node that doesn't exist raises a BadAddress exception.

Proposed Python Implementation Snippet

from functools import cache

class Node:
    # Existing implementation...

    @cache
    def get_address(self, *args):
        node = self
        for arg in args:
            if isinstance(node, Map) and isinstance(arg, str):
                node = node.data.get(arg)
            elif isinstance(node, Sequence) and isinstance(arg, int):
                try:
                    node = node.data[arg]
                except IndexError:
                    raise BadAddress(f"Index out of range: {arg}")
            else:
                raise BadNode(f"Invalid type or index: {arg}")

            if node is None:
                raise BadAddress(f"Address not found: {args}")

        return node

Discussion Points

Action Items

Notes

exfret commented 8 months ago

I added a commit for the get_addr function. There are some differences with this and the spec you outlined.

  1. The signature includes two named arguments, a list of strings and ints for the full address, and the place in the list that we are at. Thus, at each stage, we know the full address we are indexing into rather than the address local to this node, which may help with debugging.
  2. I didn't add caching. I'm not sure what it does, but it seems it's pretty simple if you do want to add it.
  3. BadNode ended up not being used. I found all errors were able to be chalked up to a bad address so far, and I don't do type checking when getting an address to make sure a node is of a correct form (and I think we should keep it that way).
  4. The test cases were slightly reorganized to fit the nonexistence of BadNode and also as I saw fit. I included a massive complex node for testing, but I think it's too big; we can trim it down, but it's working so I'm leaving it for now.
  5. I test for exceptions in a different way and raise different ones. However, I think the way I did it is just as comprehensive, and I think I also wrote comprehensive test cases so that we can be sure that my way is working.

Additionally, I made some changes in syntax and test_syntax to accommodate the new addressing tests and functionality.

  1. Map nodes used to have a getitem method. This has been removed and completely relocated to the base class Node.
  2. I changed some imports to import everything. Notably, test_syntax just imports everything from syntax. We can change it, but it seemed we would end up importing everything evenutally anyways.
  3. I added the new exceptions BadNode and BadAddress. As said, BadNode is not currently used due to seeing now that addressing problems are usually a problem with the address unless more extensive type checking is done (which I don't think is the addresser's responsibility). However, it may come in handy in the future.