ExpDev07 / coronavirus-tracker-api

🦠 A simple and fast (< 200ms) API for tracking the global coronavirus (COVID-19, SARS-CoV-2) outbreak. It's written in python using the 🔥 FastAPI framework. Supports multiple sources!
https://coronavirus-tracker-api.herokuapp.com
GNU General Public License v3.0
1.59k stars 323 forks source link

Builder Pattern For Return Data in V2.py #426

Closed MikePresman closed 3 years ago

MikePresman commented 3 years ago

Hello,

Today my PR consists of my implementation of a Creational Design Pattern known as the "Builder Pattern".

What Are The Changes? Currently, for the endpoints located in 'v2.py' the way large aggregate json data is returned is as such

  return {
        "latest": {
            "confirmed": sum(map(lambda location: location.confirmed, locations)),
            "deaths": sum(map(lambda location: location.deaths, locations)),
            "recovered": sum(map(lambda location: location.recovered, locations)),
        },
        "locations": [location.serialize(timelines) for location in locations],
    }

This is slightly messy and inconsistent when it comes to adding aggregate data over the long run. As such, I have decided to make the following changes in lieu of the Builder Pattern.


  return Data(locations)\
                        .build_deaths()\
                        .build_recovered()\
                        .build_confirmed()\
                        .get_return()

This allows for a more clean, concise and readable format where any novice developer in this repository can quickly gauge what is going on.

Potential Concerns Now, one might think there is a limitation because the dictionary is pre-constructed to follow the builder pattern. I have avoided this by allowing a generic method to take in any key and value to be added to the dictionary which also follows the builder pattern.

This method is

    def build_generic(self, key, data):
        self.__data[key] = data
        return self

As you can see any key and data can be passed and added to the dictionary while also following the builder approach. This can also be seen concretely in the endpoint "/locations" where a generic 'locations' data is added.

  return Data(locations)\
                        .build_deaths()\
                        .build_recovered()\
                        .build_confirmed()\
                        .build_generic("locations", [location.serialize(timelines) for location in locations])\
                        .get_return()

As such I strongly believe accepting this PR would benefit this repo not only today but in the long run.

Regards, Mike

Kilo59 commented 3 years ago

Net increase in lines of code with no obvious benefit to code readability or complexity. Not interested in implementing various architectural patterns just for the sake of it.