0

This question is more of a generic one. I'm starting with object programming and am quickly getting lost. It's working but I feel that I am only lucky that it is because my code is not clean...and it'll not be scalable

  1. can I leave this.rgb in the player construction function without defining it right away ? Would it be cleaner to create an "rgb function" that calculates the value (this.rgb = rgb)
  2. is it disgusting to define the rgb variable inside the newButton function (which is kind of supposed to be about something else)
  3. Do I really have to do this.newButton = newButton; (as seen on CDN) or is there a simpler/cleaner way ?
  4. Bonus : I read mike and marks explanations on templates but I feel like I can do things quicker (and dirtier) with this very long string defining the button... how dirty is it ?
/*------------------------------------------

        Player definition   
        
-------------------------------------------*/

function newButton() {
    
    r = Math.floor(Math.random() * 100 + 100)
    g = Math.floor(Math.random() * 100 + 100)
    b = Math.floor(Math.random() * 100 + 100)
    this.rgb = "rgb(" + r + ","  + g + "," + b + ")";
    
    return button = `<button onclick="playerTimer(${this.playerNumber})" class="btn" style="background-color:${this.rgb}"><h3>Player ${this.playerNumber+1} </h3> <br><br> Time left:<h2><div id="playerTimerDiv${this.playerNumber}"> 30 </div></h2></button>`;
}

function Player(playerNumber, timeout, sec) {
  this.playerNumber = playerNumber;
  this.rgb
  this.timeout = timeout;
  this.sec =sec;
  this.div = document.createElement("div");
  this.newButton = newButton;
}

Thank you very much in advance for your help ! Sorry for this very long question....

Best regards,

djspatule
  • 11
  • 5
  • 2
    This question is more suitable for [Code Review](https://codereview.stackexchange.com/), but please check their rules for good questions. – trincot Sep 29 '21 at 20:44
  • The `newButton` method really should be on the prototype, not on the instance. Why not use [`class`](//developer.mozilla.org/docs/Web/JavaScript/Reference/Classes) syntax instead? Inline event handlers like `onclick` are [not recommended](/q/11737873/4642212). They are an [obsolete, hard-to-maintain and unintuitive](/a/43459991/4642212) way of registering events. Always [use `addEventListener`](//developer.mozilla.org/docs/Learn/JavaScript/Building_blocks/Events#inline_event_handlers_%E2%80%94_dont_use_these) instead. – Sebastian Simon Sep 29 '21 at 20:46
  • 1
    `this.rgb` in `Player` does nothing. Do you want to assign a value or call a function there? Looks like a typo and rest of the line is missing. – jabaa Sep 29 '21 at 20:55

0 Answers0