cyperdark / osu-api-extended

Package for advanced work with "osu" api
MIT License
47 stars 16 forks source link

Suggestion: Refactor Function Signatures to Accept Object Parameters #15

Closed GabuTheDev closed 9 months ago

GabuTheDev commented 1 year ago

Description

Currently, many functions accept multiple parameters, which can lead to verbose function calls and potential issues with parameter order. To improve the flexibility and readability of the code, consider refactoring these funtions to accept object parameters instead.

Here are some benefits of upgrading:

Examples:

A pretty good example is the auth function.

import { auth } from "osu-api-extended";

// before
await auth.login(client_id, client_secret); 

// after
await auth.login(method, {
    client_id: 0,
    client_secret: 0,
    scopes: []
}); 

with this kind of upgrade, you can also opt-in for a single auth function.

You can now merge auth.login(), auth.login_lazer() & auth.authorize_cli() into 1 function:

const method = "lazer"; // could be either "stable", "lazer" or "cli"
data = {
    client_id: 0,
    client_secret: 0,

    login: 0,
    password: 0,

    callbackUrl: 0,
    scopes: [ 0, 0, 0 ]
};

meaning:

or you could give up on the method property entirely. here"s a code example:

async login(method, data) => {
    if (method === "stable") {
        // perform "stable" auth...
    } else if (method === "lazer") {
        // perform "lazer" auth...
    } else if (method === "cli") {
        // perform "cli" auth...
    } else thow new Error("something went wrong");
}
GabuTheDev commented 1 year ago

I am sorry if this does not fit your codebase, I wanted to give it a try and code this but I found myself lost in the files.

yorunoken commented 11 months ago

seems easy enough to implement?

type parametersType = { clientId?: number, clientSecret?: string, redirectUri?: string, scope?: auth_scopes, state?: string, username?: string, password?: string }
type methodType = "lazer" | "stable" | "cli"

export async function login(method: methodType, { clientId, clientSecret, redirectUri, scope, state, username, password }: parametersType){
  switch(method){
    case "lazer":
      return (await login_lazer(username, password))
    case "stable":
      return (await login_stable(clientId, clientSecret, scope))
    case "cli":
      return (await authorize_cli(clientId, clientSecret, redirectUri, scope, state))
    default:
      throw new Error("blah b lah...")
  }
}
yorunoken commented 11 months ago

will PR when I catch a break. Writing this from my work computer XD Also would be useful to add in checks to make sure parameters are filled appropriately

cyperdark commented 9 months ago

like that ? https://github.com/cyperdark/osu-api-extended/commit/4be0e033b7d5454cd779e6ad4f99c1cc96da9930

GabuTheDev commented 9 months ago

like that ? https://github.com/cyperdark/osu-api-extended/commit/4be0e033b7d5454cd779e6ad4f99c1cc96da9930

My initial proposal was to have the method value detached from the passing object, as a different parameter.

cyperdark commented 9 months ago

It's done like that to have autocomplition like this

https://github.com/cyperdark/osu-api-extended/assets/21735205/351c9232-3e3d-42dd-9d70-32321dd23d13

yorunoken commented 9 months ago

you can technically still have that without including it in the object. Most IDEs allow you to simply click ctrl+space to let you autocomplete

GabuTheDev commented 9 months ago

yeah but: image

image

yorunoken commented 9 months ago

yea, that's what I was saying. btw your gif doesn't work

GabuTheDev commented 9 months ago

yea, that's what I was saying. btw your gif doesn't work

it doesn't load? lemme upload it again: ezgif-2-c937a5f533

yorunoken commented 9 months ago

works now yea

cyperdark commented 9 months ago

@GabuTheDev look at video again, after i specify a method autocomplition will show parameters that can be specified for this method, rest will be hidden

GabuTheDev commented 9 months ago

@GabuTheDev look at video again, after i specify a method autocomplition will show parameters that can be specified for this method, rest will be hidden

Whatever, just push this pr.