Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Base URL should be a readonly field #45

Open
pajohns opened this issue Dec 8, 2023 · 0 comments
Open

Base URL should be a readonly field #45

pajohns opened this issue Dec 8, 2023 · 0 comments

Comments

@pajohns
Copy link
Contributor

pajohns commented Dec 8, 2023

Currently the Pinch.SDK.PinchApi constructor alters the base URI of the PinchApiOptions class that is being passed in through the following lines.

if (!string.IsNullOrEmpty(options.BaseUri))
        options.BaseUri = options.BaseUri.TrimEnd(new char[1]
        {
          '/'
        }) + "/" + (options.IsLive ? "live" : "test") + "/";
      else
        options.BaseUri = options.IsLive ? "https://api.getpinch.com.au/live/" : "https://api.getpinch.com.au/test/";

I have created a service that instantiates the PinchApiOptions in its constructor and is set as a readonly object. My class contains multiple methods, each of which pass the options into the PinchApi constructor. Given the PinchApi constructor is altering the BaseUri this is what happens.

Initially
When first created, the BaseUri is null as I hadn't configured it.

After one instantiation of PinchApi
The BaseUri is modified to "https://api.getpinch.com.au/test/"

After an additional instantiation of PinchApi
The BaseUri is modified to "https://api.getpinch.com.au/test/test/"
which results in a 404 not found when attempting to calling my method.

I suggest moving the logic from the API constructor into the constructor of the PinchApiOptions which will set the default values. I would also set all the values to readonly { get; } meaning that after they've been set initially they can't be changed.

pajohns added a commit to pajohns/Pinch.SDK that referenced this issue Dec 8, 2023
…members to be set during construction only.
dkarzon added a commit that referenced this issue Dec 19, 2023
Resolve issue #45 - PinchApiOptions to be readonly
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant