calpoly-csai / calpoly-csai/api

Formatting Refactor Discussion

Open
#174 3 comments 0 reactions 5 assignees Claimed by @cameron-toy View on GitHub
enhancement
Dominant language
Python
Stars
9
Forks
4
PR merge metrics
No merged PRs in 30d

Description

## Objective
Reduce the scope of the database wrapper to interacting with the database.

## Proposal
Require that all Entities have a validator and formatter class attached. By updating this class, we can make our error checking extensible and move most of the logic out of the ``database_wrapper.py`` file.

We have a top level initialization static method on each class called ``from_dict(data)``. All of the validation/formatting code in the ``database_wrapper`` could be reduced to the following:
```
try:
entity = entity_type.from_dict(data)
except:
#Handle errors
#Perform insert/update/delete
```

Internally this method would incorporate the following:

```
validator = EntityTypeValidator()
formatter = EntityTypeFormatter()
validator.validate(data) #Checks data for critical errors, raises exceptions handled above.
formatted_data = formatter.format(data) #Casts data to correct type and fills in missing replaceable fields.
entity = EnitityType()
for key in formatted_data:
setattr(entity, key, formatted_data[key])
return entity
```

Examples of validators and formatters can be found the the ``./modules`` folder.

## Benefits
- Upholds single responsibility principle: database_wrapper is focused on interacting with database. Formatters format. Validators validate
- Extensible: allows formatting and validating to be abstracted from the wrapper, so adding new features doesn't mean pushing breaking changes.
- Modularized: Allows code to be divided into different files, making the codebase more readable.
- More performant? We're loading k/v pairs for specific use cases every time we init the server. If this were abstracted, we wouldn't have to load overhead for functionalities outside of the scope of a specific REST API endpoint.

## Details
Currently the database has formatting and validation functionality baked into the wrapper. The code rigidly enforces that each ``Entity`` adheres to the keys defined in ``EXPECTED_KEYS_BY_ENTITY``, but there are cases for the Nimbus Validator App where an ID needs to be passed instead of automatically generated. If I were to change the code in the ``validate_and_format_entity_data()`` function, there would likely be other breaking changes.

The proposed format would allow me to edit the ``QuestionAnswerPair`` formatter in isolation in order to field this new use case.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.