amadeus4dev / amadeus-java

Java library for the Amadeus Self-Service travel APIs
https://developers.amadeus.com/
MIT License
87 stars 69 forks source link

com.amadeus.resources.Traveler has inconsistencies between constructors #196

Closed steve-donovan closed 2 years ago

steve-donovan commented 2 years ago

Description

  1. Traveler constructor only sets a subset of the class members even though all are provided in the method signature.

    public Traveler(String id, String dateOfBirth, String gender,
      Name name, Contact contact, Document[] documents) {
    this.name = name;
    this.contact = contact;
    }
  2. Traveler.Contact parameterized constructor has no body

    public Contact(Phone phones, String emailAddress) {
    }
  3. Traveler.Document parameterized constructor has no body

    public Document(String documentType, String number, String expiryDate, 
        String issuanceCountry, String nationality, boolean holder) {
    
    }
  4. Traveler.Phone parameterized constructor has no body

    public Phone(String countryCallingCode, String number) {
    }

Should I add appropriate bodies, drop the parameterized constructors completely and rely on setter usage? What are the mandatory fields, and how to enforce them if only using setters -rely purely on marshaling?

Let me know and I'll raise a PR.

jabrena commented 2 years ago

Hi @steve-donovan.

good review, go ahead with the PR.

Juan Antonio

steve-donovan commented 2 years ago

Hi @jabrena Do you think we should remove the parameterised constructors and rely on setters, or complete the constructor bodies?

jabrena commented 2 years ago

I will review the class and I will say something tomorrow.

Where is you come from? here, it is 15:00 CEST

and you?

steve-donovan commented 2 years ago

Thanks @jabrena I'm from UK and there visiting family at the moment, but I actually live in France currently. I recently read about a new digital nomad visa for Bali - 5 years and tax-free. So I may say goodbye to France 😊

jabrena commented 2 years ago

I will help you but please, share a pic from Bali one day! :p

I reviewed the class and honestly, we need to do some changes:

I am going to think about it, tomorrow we will continue this conversation.

steve-donovan commented 2 years ago

hahaha happily

oh yes i see - that's a little bit ugly. Looks like some debt between Traveler and FlightOrder.Traveler.

ok talk later. thanks.

tsolakoua commented 2 years ago

Thanks for reporting the issue @steve-donovan, I will check it out as well and create a PR soon.