Subterfuge-Revived / Remake-Backend

Server side validation and API
Creative Commons Zero v1.0 Universal
10 stars 1 forks source link

Consistent Responses #9

Closed QuinnBast closed 4 years ago

QuinnBast commented 4 years ago

All endpoints should return consistent responses. This is related to #10.

This is best explained with an example. Right now get_room_data can respond with:

[ list_of_room_events ]

Or, it can respond with:

{"success":false,"message":"[new_room] Invalid session. Authentication required"}

The first response is a list, the second response is an object. As suggested in #10, all responses should be an object. Having inconsistent return types makes it near impossible for me to consistently parse the data without knowing in advance that the request is going to fail or succeed. Because of this I have to use a try catch which is not ideal...

try {
    gameEvents = JsonConvert.Deserialize<List<GameEvent>>(networkResponse);
} catch (ParseException e) {
    networkFailure = Json.Convert.Deserialize<GameEventListFailure>(networkResponse);
}

Ideally, all responses back from the network should have a consistent format so that parsing the response doesn't differ. For example, something like this could be nice:

{
    success: true,
    message: "Success",
    value: {}, // Data to return on success. Could be object, array, whatever you want here. null if failure.
}

In PHP like this:

// Base class:
class Response {
    public $success;
    public $message;
    public $value;

   public toJSON() {
       return json_encode($this);
   }
}

// Classes for success and failure:
class SuccessResponse extends Response {
    public function __construct($value) {
        $this->success = true;
        $this->message = "Success";
        $this->value = $value
    }
}

// Easily output formatted responses:
echo new SuccessResponse($gameEventList).toJSON();

This would make parsing responses consistent as everything is an object, and everything will fit into a default class. I could easily parse the default class, check if(response.success) in order to determine if I can go ahead and parse the value.

QuinnBast commented 4 years ago

This may not be required after the move to laravel

QuinnBast commented 4 years ago

Resolved with the database models implemented in the laravel implementation on #10