Search Unity

am i coding the right way?

Discussion in 'Scripting' started by Landci, Feb 3, 2016.

  1. Landci

    Landci

    Joined:
    Aug 28, 2012
    Posts:
    33
    i often have situations like the one below.
    where i need the same function only with minor differences.
    i always end up just copy and pasting.
    But i wonder if there is not a better solution for that?:confused:


    Code (CSharp):
    1. public void Item0Pressed(){
    2.         if (niete0 ==0 || niete1 ==0){
    3.            
    4.             if (Item0.GetComponentInChildren<ParticleSystem>().isPlaying){
    5.                 Item0.GetComponentInChildren<ParticleSystem>().Stop();
    6.                 Item0.GetComponentInChildren<ParticleSystem>().Clear();}
    7.            
    8.             Item0.GetComponentInChildren<ParticleSystem>().Play();
    9.             Item0.GetComponentInChildren<Animator>().enabled= false;
    10.             nieten++;
    11.             if (nieten ==2)
    12.                 BonusGameOver();
    13.        
    14.         }else{
    15.            
    16.             Item0.GetComponentInChildren<Animator>().SetTrigger("animateWin");
    17.             totalWon= totalWon+item0Value;
    18.             TotalWon.GetComponent<UILabel>().text=totalWon.ToString();
    19.             Invoke("Item0AnimOff",1.1f);
    20.         }
    21.  
    22.         Item0.GetComponent<UI2DSprite>().enabled= false;
    23.     }
    24.     public void Item1Pressed(){
    25.         if (niete0 ==1 || niete1 ==1){
    26.            
    27.             Item1.GetComponentInChildren<ParticleSystem>().Play();  
    28.             Item1.GetComponentInChildren<Animator>().enabled= false;
    29.             nieten++;
    30.             if (nieten ==2)
    31.                 BonusGameOver();
    32.         }else{
    33.             Item1.GetComponentInChildren<Animator>().SetTrigger("animateWin");
    34.             totalWon= totalWon+item1Value;
    35.             TotalWon.GetComponent<UILabel>().text=totalWon.ToString();
    36.             Invoke("Item1AnimOff",1.1f);
    37.         }
    38.  
    39.         Item1.GetComponent<UI2DSprite>().enabled= false;
    40.     }
    41.     public void Item2Pressed(){
    42.         if (niete0 ==2 || niete1 ==2){
    43.            
    44.             Item2.GetComponentInChildren<ParticleSystem>().Play();
    45.             Item2.GetComponentInChildren<Animator>().enabled= false;
    46.             nieten++;
    47.             if (nieten ==2)
    48.                 BonusGameOver();
    49.         }else{
    50.            
    51.             Item2.GetComponentInChildren<Animator>().SetTrigger("animateWin");
    52.             totalWon= totalWon+item2Value;
    53.             TotalWon.GetComponent<UILabel>().text=totalWon.ToString();
    54.             Invoke("Item2AnimOff",1.1f);
    55.         }
    56.  
    57.         Item2.GetComponent<UI2DSprite>().enabled= false;
    58.     }
    59.     public void Item3Pressed(){
    60.         if (niete0 ==3 || niete1 ==3){
    61.            
    62.             Item3.GetComponentInChildren<ParticleSystem>().Play();
    63.             Item3.GetComponentInChildren<Animator>().enabled= false;
    64.             nieten++;
    65.             if (nieten ==2)
    66.                 BonusGameOver();
    67.         }else{
    68.            
    69.             Item3.GetComponentInChildren<Animator>().SetTrigger("animateWin");
    70.             totalWon= totalWon+item3Value;
    71.             TotalWon.GetComponent<UILabel>().text=totalWon.ToString();
    72.             Invoke("Item3AnimOff",1.1f);
    73.         }
    74.  
    75.         Item3.GetComponent<UI2DSprite>().enabled= false;
    76.     }
    77.     public void Item4Pressed(){
    78.         if (niete0 ==4 || niete1 ==4){
    79.            
    80.             Item4.GetComponentInChildren<ParticleSystem>().Play();
    81.             Item4.GetComponentInChildren<Animator>().enabled= false;
    82.             nieten++;
    83.             if (nieten ==2)
    84.                 BonusGameOver();
    85.         }else{
    86.             Item4.GetComponentInChildren<Animator>().SetTrigger("animateWin");
    87.             totalWon= totalWon+item4Value;
    88.             TotalWon.GetComponent<UILabel>().text=totalWon.ToString();
    89.             Invoke("Item4AnimOff",1.1f);
    90.         }
    91.  
    92.         Item4.GetComponent<UI2DSprite>().enabled= false;
    93.     }
     
  2. orb

    orb

    Joined:
    Nov 24, 2010
    Posts:
    3,038
    Remember that methods can take parameters :)

    You can probably boil it all down to one 15-line (or shorter) method that takes two arguments (an integer and the object with the sprite to disable).
     
  3. TonyLi

    TonyLi

    Joined:
    Apr 10, 2012
    Posts:
    12,706
    You have a good instinct to suspect that something is wrong. An important programming principle is DRY, or "Don't Repeat Yourself."

    C# offers many complex techniques such as generic methods and delegates. But you don't need them for this. You just need to pass parameters to your methods and make them more in a more general-purpose manner. [Just as orb posted while I was writing this. ]

    For a really basic example, say you've written methods like this:

    Code (csharp):
    1. int AddOnePlusOne() {
    2.     return 1+1;
    3. }
    4.  
    5. int AddOnePlusTwo() {
    6.     return 1+2;
    7. }
    8.  
    9. int AddTwoPlusTwo() {
    10.     return 2+2;
    11. }
    12. //etc.
    But it's much easier to do this:

    Code (csharp):
    1. int Add(int a, int b) {
    2.     return a+b;
    3. }
    So in your case you could make a general-purpose method that looks something like this (approximately):

    Code (csharp):
    1. public void ItemPressed(GameObject item, int requiredValue, int itemWinValue){
    2.     if (niete == requiredValue) {
    3.         DisableItem(item);
    4.         nieten++;
    5.         if (nieten == 2) BonusGameOver();
    6.     } else {
    7.         SetWinState(item, itemWinValue);
    8.     }
    9.     Item0.GetComponent<UI2DSprite>().enabled= false;
    10. }
    11.  
    12. public void DisableItem(GameObject item) {
    13.     if (item.GetComponentInChildren<ParticleSystem>().isPlaying) {
    14.         item.GetComponentInChildren<ParticleSystem>().Stop();
    15.         item.GetComponentInChildren<ParticleSystem>().Clear();
    16.     }
    17.     item.GetComponentInChildren<ParticleSystem>().Play();
    18.     item.GetComponentInChildren<Animator>().enabled= false;
    19.     // Also, should really get component once and then re-use its reference.
    20. }
    21.  
    22. public void SetWinState(GameObject item, int itemWinValue) {
    23.     item.GetComponentInChildren<Animator>().SetTrigger("animateWin");
    24.     totalWon= totalWon+itemWinValue;
    25.     TotalWon.GetComponent<UILabel>().text=totalWon.ToString();
    26.     Invoke("Item0AnimOff",1.1f);
    27. }
    The code above is a rough idea. I don't know exactly what your code is supposed to do, but I think this should get the idea across. Then you can use it like this:

    Code (csharp):
    1. ItemPressed(Item4, 4, item4Value);
    There's more you can do to reuse code even further, but this would be a good start.
     
    Nigey likes this.
  4. orb

    orb

    Joined:
    Nov 24, 2010
    Posts:
    3,038
    @TonyLi made a reasonable example in the expected 15 lines - do something like that or even collapse it all into one method :)