1
\$\begingroup\$

I am seeking a general code review.

Task:
Write a program that takes JSON-formatted opening hours of a restaurant as an input and outputs hours in more human readable format.

Input:
Input JSON sconsist of keys indicating days of a week and corresponding opening hours as values. One JSON file includes data for one restaurant.
{
[dayofweek]: [opening hours]
[dayofweek]: [opening hours]
}
: monday / tuesday / wednesday / thursday / friday / saturday / sunday
: an array of objects containing opening hours. Each object consists of two keys:

  • type: open or close
  • value: opening / closing time as UNIX time (1.1.1970 as a date),
    eg. 32400 = 9 AM, 37800 = 10:30 AM
    max value is 86399 = 11:59:59 PM

The full spec for this assignment in PDF format and sent this solution I used Pydantic and FastAPI for fancy style but it was a very bad decision since I don't use any benefit of those frameworks but my main thing is with the logic of course.

It is disliked and I understand why it is disliked and improved some parts.

However two issues that I understand but could not improve or find a better solution is listed below. Cause I thought what I did was clever solution but I also understand it is mixing some stuff and making it harder than it should but this was the best that I could come up with so that is why I am asking review.

  • The formatting of the results is intermixed with business logic making extending the code harder than necessary.
  • The main for loop is filled with mutating data and extra control structures. To improve, think how to write it without using "continue", or mutating data structures.

models.py

from typing import List, Optional, Literal

from typing_extensions import TypedDict
from pydantic import BaseModel


class OpeningHours(TypedDict):
    """
    Typed Dict model for the opening hours info that we receive from the restaurants.
    """

    type: Literal["open", "close"]
    value: int


class OpenDays(BaseModel):
    """
    Pydantic model "/opening_info" endpoint requires these fields for post request.
    """

    monday: List[Optional[OpeningHours]]
    tuesday: List[Optional[OpeningHours]]
    wednesday: List[Optional[OpeningHours]]
    thursday: List[Optional[OpeningHours]]
    friday: List[Optional[OpeningHours]]
    saturday: List[Optional[OpeningHours]]
    sunday: List[Optional[OpeningHours]]

data_parser.py

from datetime import datetime
from typing import Dict, List, Any


def convert_unix_to_hour(unix_time: int) -> str:
    """
    Converts unix time to datetime and returns 12-hour clock format from it.
    :param unix_time: unix time stamp in integer format
    :return: str: 12-hour clock format i.e. 9 AM
    """
    time_format = "%I:%M %p"

    return datetime.fromtimestamp(unix_time).strftime(time_format)


def get_hours_from_sorted_list(sorted_list: List) -> str:
    """
    Gets the unix hours data from the given list of dicts.
    :param sorted_list: sorted version of models.OpeningHours
    :return: str: opening and closing hours of the restaurant joined with " - " i.e. "9 AM - 11 AM"
    """
    hours_list = []
    for item in sorted_list:
        hours_list.append(convert_unix_to_hour(item["value"]))
    return " - ".join(hours_list)


def parse_restaurant_data(open_days: Dict[str, Any]) -> Dict[str, str]:
    """
    Parses restaurant data that comes from "/opening_info" endpoint in to more human readable way.
    This algorithm has two conditions. Firstly if there is no opening info for that day,
    restaurant's formatted_date is set to "Closed" for the day. Each restaurant opening_info is
    sorted by their time to make sure they are not mixed, if the first type of the opening_info's
    is "open", it is being added to the formatted_date as normally.
    However, if this two conditions didn't met it means that info is "closed" and the time is
    being added at the end of to the previous day's formatted_date.
    :param open_days: OpenDays model in dictionary type.
    :return: formatted_dates: human readable version of what "/opening_info" endpoints receives.
    """
    formatted_dates = {
        "monday": "Closed",
        "tuesday": "Closed",
        "wednesday": "Closed",
        "thursday": "Closed",
        "friday": "Closed",
        "saturday": "Closed",
        "sunday": "Closed",
    }
    days = list(formatted_dates.keys())

    for day, opening_info in open_days.items():
        if not opening_info:
            continue

        sorted_opening_info = sorted(opening_info, key=lambda time: time["value"])

        if sorted_opening_info[0]["type"] == "open":
            hours = get_hours_from_sorted_list(sorted_opening_info)
            formatted_dates[day] = hours
            continue

        closing_hour_of_previous_day = convert_unix_to_hour(sorted_opening_info[0]['value'])

        index_of_today = days.index(day)
        previous_day = days[index_of_today - 1]

        hours_of_previous_day = formatted_dates[previous_day]

        formatted_dates[previous_day] = f"{hours_of_previous_day} - {closing_hour_of_previous_day}"
        sorted_opening_info.pop(0)

        hours = get_hours_from_sorted_list(sorted_opening_info)
        formatted_dates[day] = hours

    return formatted_dates

main.py

from fastapi import FastAPI
from starlette.responses import JSONResponse

from date_parser import parse_restaurant_data
from models import OpenDays

app = FastAPI()


@app.post("/opening_info")
def opening_info(open_days: OpenDays):
    """
    Post endpoint to receive restaurants opening and closing info.
    Requires OpenDays model in body parse this data in to more human readable way with the
    date_parser.parse_restaurant_data and returns it as a starlette.responses.JSONResponse (FastAPI practice)
    :param open_days: pydantic OpenDays model
    :return: JSONResponse
    """
    open_days_as_dict = open_days.dict()

    parsed_restaurant_data = parse_restaurant_data(open_days_as_dict)

    return JSONResponse(content=parsed_restaurant_data)

Any help is appreciated about data mutation or about cleaner approach and etc.

\$\endgroup\$
6
  • 4
    \$\begingroup\$ We can provide a better code review when more of the code is included. \$\endgroup\$
    – pacmaninbw
    Oct 5, 2021 at 15:43
  • \$\begingroup\$ @pacmaninbw hey I shared the github link actually but I also had to put at least 3 lines of code so I just copied main loop. My bad sorry, here is the link again. github.com/mrlkn/wolt-assignment \$\endgroup\$
    – Omer Alkin
    Oct 5, 2021 at 16:08
  • 5
    \$\begingroup\$ We can only review the code embedded in the question itself. \$\endgroup\$
    – pacmaninbw
    Oct 5, 2021 at 16:27
  • \$\begingroup\$ Can you provide the functions missing, like convert_unix_to_hour and get_hours_from_sorted_list? \$\endgroup\$
    – Linny
    Oct 8, 2021 at 9:35
  • \$\begingroup\$ I am terribly sorry didn't read the rule about only embedded code. @pacmaninbw \$\endgroup\$
    – Omer Alkin
    Oct 13, 2021 at 14:28

0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.