Search code examples
javascriptsolid-principlessingle-responsibility-principle

Where to instantiate my classes and still adhere to SRP


My questions is specific to adhering to SRP on the client side where we need to maintain state in our objects instead of adhering to SRP on the server where our state is maintained within a database, which I will post a new question for to shorten this one. This question is full of smaller questions that I dont expect a direct answer to everything , but are mainly asked to explain a thought process.

Out of all of the SOLID principles this is the one I have read up on the most and I simply get confused every time. It’s just not clicking, but not for the want of trying. Ive read the generic ‘it has to have one responsibility’ to ‘it only has one reason to change’ and ‘it is meant to be respond to a single actor’. Then I think it makes sense and I put it into practice and it just gets even more confusing following these quotations.

Here is the summary of what im thinking :

Within a single class as long as the properties all represent the entity we are describing and as long as there are a) not several methods targeting one specific property or b) a group of properties that relate to each other instead of being a standalone property describing the person start to form then we are adhering to SRP.

However when these issues in a or b start to form then we need to refactor to adhere to SRP. Similar to how we create new tables within a mysql database when a column relates to something else other than just the primary key then in a normalised database we would create a new table linking both together through keys. In OOP , at this point we create separate classes for each responsibility?

Let me explain my thought process Example 1 we could store everything within the same class like this, this for me violates SRP

class Person {
  constructor(name,streetName,doorNum){
    this.name = name;
    this.streetName = streetName;
    this.doorNum = doorNum;
  }
  getAddress(){
    return {
    streetName: this.streetName,
    doorNum: this.doorNum  
    }
  }
  changeName(newName){
    this.name = newName;
  }
  changeDoorNumber(num){
    this.doorNum = num;
  }
  getName(){
    return this.name;
  }
}

Although each property plays a part in describing the Person which is the target entity, both the name and the address can be encapsulated within its own class like this.

class Person {
  constructor(name,streetName,doorNum){
    this.name = new Name(name)
    this.address = new Address(streetName,doorNum)
  }
  getAddress(){
    return this.address.getAddress();  
  }
  changeDoorNumber(num){
    this.address.changeDoorNumber(num)
  }
  getName(){
    this.name.getName()
  }
  changeName(newName){
    this.name.changeName(newName)
  }
}

Here we have methods within the Person class that calls methods from the Name and Address classes, but the properties and methods of the Name and Address classes are encapsulated within their own class and supply their own interface to be used within the person class.

This leads me to think where do I instantiate these separate classes effectively to relate them to the entity they refer to. Above I have stored the Address and Name class within the Person class by instantiating the classes within the Person class which I like to refer to as the HOST class. I am also aware I could injection an instantiation of them when creating the object.

However the Person class itself still has the same amount of methods in it, it just doesn’t directly contain the logic of the method and instead just calls methods from the classes within it. Is this ok or shall I be instantiating the person class which instantiates the Name and Address class and then when I want to call a method of the name and address classes then do something like person.name.changeName() ? The Address and Name classes still need to refer to a specific person somehow right so must be instantiated within the Person class ?

Alternatively I could seperate all of these classes entirely and link each association through a common Id, kind of like how a mysql database relational pattern would work. See Below

class Person {
  constructor(id, name ){
    this.id = id;
  }
  getId(){
    return this.id;
  }
}

class Address {
  constructor(person_id, streetName, doorNum){
    this.person_id = person_id;
    this.streetName = streetName;
    this.doorNum = doorNum;
  }
  getAddress (){
    return this;
  }
}

Then do the same with the Name class. Here I could retrieve the address and name through the Id of the person and I wouldn’t then have to bloat my Person class with methods just to keep together classes that belong within the specific person. I haven’t seen this practice client side before which makes me think that this is not a thing?

The next part is the part that confuses me now the most and an answer to this would really help -

So far within these classes I am targeting specific details of each person , name or address. However now I don’t want to carry out a piece of functionality by using the Person details rather than just changing or getting them. Let’s say the Person wants to apply for a bank loan. I want to send a request to the server whom deals with the request and business logic as to whether the person will be successful in his loan application. It’s a relatively small loan and just requires the person name and address to decide upon approval or not. Am I correct in saying that this Loan class doesn’t have to be within the Person class because I can retrieve the name and address from the Person class and pass it to the Loan class? However if I wanted to update a property within the loan class such as doesHaveALoan then now I would need that Loan class to be relatable to a specific person again to so I would move instantiate it or pass it into the players class again because it needs to relate to a specific player? Alternatively I could pass the instantiated Loan object that stores the doesHaveALoan property into a mediator aswell as a second class such as LoanApplication and complete the mediation within there which may mean calling a method to update the Loan class within the specific player . Is this correct or have I misunderstood?

Rather than answering all my questions above I have included them to explain my thought process.

The things I would like an answer to are

  1. Should I couple my separate Name and Address classes within the person class or associate them through a common id ? 2)Does the Location of the instantiation of my Loan class depend on whether I want to have a property within the class that is coupled to the person making the loan as described above
  2. Shall I create a new class as soon as I have more than one method targeting a property and that property and methods dont work in tandem with other methods or properties within this class as shown above ?

Solution

  • From my above comments ...

    The entire approach already is questionable due to public accessible properties plus additional get/set methods is. With public properties there is no need of additional get/set functionality. The latter is strongly advised for controlling access of privately held properties. – Peter Seliger

    @PeterSeliger so I should just directly use the property name instead of a get method ? Or use the get method and set the props as private? – Kevin Greetham

    @KevinGreetham ... it, as always, depends from how much you want to enforce/guarantee controlled value changes of each property. Foolproof is an implementation of true privacy everywhere with get/set accessors. More economic might be public properties within an environment which guarantees that surrounding and consuming business logic will never mess with all the public data. – Peter Seliger

    @KevinGreetham ... a rule of thumb which one can follow is ... "As soon as implementation details of a specific domain tend to create their own specific and complex handling, it most probably can be described by a new domain specific abstraction". – Peter Seliger

    A possible implementation of the scenario described by the OP, which in addition controls how properties are accessible, could look like follows. I personally prefer lighter models, but less strictly modeled abstractions are only possible within an environment where data validity and data access are already in control by one owns business logic.

    const client = new Client({
      // - omitting entirely any address specific data.
    
      firstName: 'Ælfflæd', middleName: 'Cenhelm', familyName: 'Æthelflæd',
    });
    console.log('client.valueOf() ...', client.valueOf());
    console.log('client.fullName ...', client.fullName);
    
    client.changeAddress({
      // - omitting `doorNum`.
    
      streetName: 'Madison Street', streetNumber: '877',
      zipCode: '02072', location: 'Stoughton',
      state: 'MA', country: 'USA',
    });
    console.log('client.address ...', client.address);
    
    console.log("try ... client.middleName = 'f  -'");
    client.middleName = 'f  -';
    .as-console-wrapper { min-height: 100%!important; top: 0; }
    <script>
    function getSanitizedName(value = '') {
      const sanitized =
        String(value).normalize('NFC').trim().replace(/\s+/g, ' ');
    
      if (
        // - change/adapt name validity test to one own needs.
    
        (sanitized === '') ||
        (/^[-]+$/).test(sanitized) ||
        !(/^\p{L}{2}[\p{L} -]{0,28}$/u).test(sanitized)
      ) {
        throw new RangeError(
          `The provided value of "${ value }" does not count as valid name`
        );    
      }
      return value;
    }
    
    class ClientAddress {
      #addressData = {
        streetName: '', streetNumber: '', doorNum: '',
        zipCode: '', location: '', state: '', country: '',
      };
    
      constructor(addressData) {
        this.compile(addressData);
      }
    
      // - sole address-data related generic set method.
    
      compile(addressData = {}) {
        const data = Object(addressData);
    
        const {
          streetName, streetNumber, doorNum,
          zipCode, location, state, country,
        } = this.#addressData;
    
        Object.assign(this.#addressData, {
    
          // - implement more strict type and/or validity checks as needed.
    
          streetName: String(data.streetName ?? '').trim() || streetName,
          streetNumber: String(data.streetNumber ?? '').trim() || streetNumber,
          doorNum: String(data.doorNum ?? '').trim() || doorNum,
          zipCode: String(data.zipCode ?? '').trim() || zipCode,
          location: String(data.location ?? '').trim() || location,
          state: String(data.state ?? '').trim() || state,
          country: String(data.country ?? '').trim() || country,
        });
      }
    
      // - sole address-data related generic get method.
    
      valueOf() {
        return { ...this.#addressData };
      }
    }
    
    class Client {
      #clientData = {
        address: null,    
        firstName: '',
        middleName: '',
        familyName: '',
      };
    
      constructor({
        firstName = '', middleName = '', familyName = '', ...restData
      }) {
        // - does already trigger the name validty check(s)
        //   due to assigning each name related values as
        //   public property to the newly created instance
        //   which invokes each corresponding `set` method.
        Object.assign(this, {
          firstName, middleName, familyName,
        });
        // - aggregate client speciffic address data which
        //   comes with its own domain specific abstraction.
        this.#clientData.address = new ClientAddress(restData);
      }
    
      // - name specific getter/setter implementation.
    
      get firstName() {
        return this.#clientData.firstName;
      }
      set firstName(value = '') {
        this.#clientData.firstName = getSanitizedName(value);
      }
    
      get middleName() {
        return this.#clientData.middleName;
      }
      set middleName(value = '') {
        this.#clientData.middleName = getSanitizedName(value);
      }
    
      get familyName() {
        return this.#clientData.familyName;
      }
      set familyName(value = '') {
        this.#clientData.familyName = getSanitizedName(value);
      }
    
      get fullName() {
        const { firstName, middleName, familyName } = this.#clientData;
        return [firstName, middleName, familyName].join(' ');
      }
    
      // - client-specific, but address-related, getter/setter
      //   implementation with forwarding to address-specific
      //   methods.
    
      get address() {
        return {
          fullName: this.fullName,
          ...this.#clientData.address.valueOf(),
        }
      }
      changeAddress(addressData) {
        this.#clientData.address.compile(addressData);
      }
    
      // - client-data related generic method(s).
    
      valueOf() {
        const { firstName, middleName, familyName } = this.#clientData;
        return {
          firstName, middleName, familyName,
          address: this.#clientData.address.valueOf(),
        }
      }
    }
    </script>