Using a method instead of constructor to initialize

Starting from:

class Prix
{
    double valeur;
    string monnaie;

    public Prix(double valeur, string monnaie)
    {
        if (monnaie.Equals("€") || monnaie.Equals("$"))
        {
            this.valeur = valeur;
            this.monnaie = monnaie;
        }
        else
        {
            defaultConstr(valeur);
        }
    }

    public Prix(double valeur)
    {
        defaultConstr(valeur);
    }

    private void defaultConstr(double valeur)
    {
        this.valeur = valeur;
        this.monnaie = "€";
    }
}

• When you have multiple constructors with similar parameters, it makes sense to keep the construction logic local to one constructor. One approach is to make one constructor call straight through to a second with any extra parameters.

  public Prix(double valeur)
      : this(valeur, "€")
  {
  }
  
  public Prix(double valeur, string monnaie)
  {
      ...
  }
  

  Alternatively, supplying a default value for optional parameters is possible:

  public Prix(double valeur, string monnaie = "€")
  {
      ...
  }
  

  (see this question for information about the difference between the two)

• Having construction logic in one place means that defaultConstr is only being called in one place. This method call can be inlined:

  public Prix(double valeur, string monnaie)
  {
      if (monnaie.Equals("€") || monnaie.Equals("$"))
      {
          this.valeur = valeur;
          this.monnaie = monnaie;
      }
      else
      {
          this.valeur = valeur;
          this.monnaie = "€";
      }
  }
  

• We're now setting valeur in both branches, so that can be factored out:

  public Prix(double valeur, string monnaie)
  {
      this.valeur = valeur;
  
      if (monnaie.Equals("€") || monnaie.Equals("$"))
      {
          this.monnaie = monnaie;
      }
      else
      {
          this.monnaie = "€";
      }
  }
  

• There is no need to compare against € if that will be assigned anyway:

  public Prix(double valeur, string monnaie)
  {
      this.valeur = valeur;
  
      if (monnaie.Equals("$"))
      {
          this.monnaie = monnaie;
      }
      else
      {
          this.monnaie = "€";
      }
  }
  

• Next, consider what happens if we have more valid currency symbols, we could end up with code like this:

  if (monnaie.Equals("$") || monnaie.Equals("£") || monnaie.Equals("¥") || monnaie.Equals("฿") || monnaie.Equals("₩"))
  {
      ...
  }
  

  This is getting quite ungainly - it's best to put this into a list that can be checked against:

  class Prix
  {
      static HashSet<string> monnaieValid = new HashSet<string>
      {
          "$" // , "£", "¥", "฿", "₩"
      };
  
      ...
  
      public Prix(double valeur, string monnaie)
      {
          this.valeur = valeur;
  
          if (monnaieValid.Contains(monnaie))
          {
              this.monnaie = monnaie;
          }
          else
          {
              this.monnaie = "€";
          }
      }
  }
  

  This list is static because we don't need every Prix object to have its own list - one shared copy is enough.

• It is also advisable to avoid "magic strings" - create a constant for the default currency and use this instead of the literal string:

  class Prix
  {
      const string MonnaieDefault = "€";
  
      ...
  
      public Prix(double valeur) 
          : this(valeur, MonnaieDefault)
      {
      }
  
      public Prix(double valeur, string monnaie)
      {
          this.valeur = valeur;
  
          if (monnaieValid.Contains(monnaie))
          {
              this.monnaie = monnaie;
          }
          else
          {
              this.monnaie = MonnaieDefault;
          }
      }
  }
  

• I would also make a few other small changes to help readability (although this step may be subjective):

  • Add access modifiers where it's currently implicit
  • Make fields readonly wherever possible - here no fields are modified after they are initialised, so they can all be made readonly
  • Use a ternary operator instead of the if statement

  Finally, the class looks like this:

  public class Prix
  {
      private const string MonnaieDefault = "€";
  
      private static readonly HashSet<string> monnaieValid = new HashSet<string>
      {
          "$"
      };
  
      private readonly double valeur;
      private readonly string monnaie;
  
      public Prix(double valeur) 
          : this(valeur, MonnaieDefault)
      {
      }
  
      public Prix(double valeur, string monnaie)
      {
          this.valeur = valeur;
          this.monnaie = monnaieValid.Contains(monnaie) ? monnaie : MonnaieDefault;
      }
  }
  

I know this is not the review you are looking for, but it must be said.

Please, avoid names in other languages.

It is really tempting to write variable and property names in other languages. But please, avoid it.

I am Portuguese, and can understand some of your code. Now imagine I want to change it, and will change it to my way.

Here is your original code with a tiny change to allow pounds(£), with a bit of Portuguese in it:

class Prix
{
    double valeur;
    string moeda;

    public Prix(double valeur, string moeda)
    {
        if (moeda.Equals("€") || moeda.Equals("$") || moeda.Equals("£"))
        {
            this.valeur = valeur;
            this.moeda = moeda;
        }
        else
        {
            defaultConstr(valeur);
        }
    }

    public Prix(double valeur)
    {
        defaultConstr(valeur);
    }

    private void defaultConstr(double valeur)
    {
        this.valeur = valeur;
        this.moeda = "€";
    }
}

"Moeda" means "coin" or "currency" in Portuguese. Is that what monnaie? Does it mean "currency"? I have no idea, but I assume it does.

Now, look at that code. What a beautiful mess! You can't understand what's there, I can't understand what's there... Nobody can understand it!

Bottom line is: Always use English names for your code. This avoids a huge linguistic bareer and helps to keep the code clean and easy-to-understand by everybody, specially those who don't speak French.

currency and value would be perfect names, if that is what valeur and monnaie mean...

Also, there's no documentation on your code. Not even a single comment, for non-French speakers. And even for French speakers!

How will I know what can I pass to the constructor if there's no documentation? How will I know that the 2nd parameter is optional? How would I know that it expects a double instead of a float?

If you use this in a project, 3 months from now, you will ask "What in the name of God this needs to run!?".

So, please, add some documentation to it. Visual Studio makes it easy to document your code. Just start by writting ///<sumary> and it will almost complete everything for you.

If you want more information, you can read @Shymep's answer on StackOverflow.

Default Parameters With all the extraneous stuff taken out, why not use default constructor with default parameter values? I haven't seen them mentioned yet.

class Prix
{
   double valeur;
   string monnaie;

   public Prix(double valeur, string monnaie = "€")
   {
      valeur = valeur;
      monnaie = monnaie;
   }
}

With default parameter values you can use the first parameter alone (implicitly using the default)... or explicitly set the Symbol to "$" (or whatever you need it to be).

Pair this with some of the other options to verify you have a valid token:

class Prix
{
   static HashSet<string> monnaieValid = new HashSet<string>
   {
      "€", "$" //, "£", "¥", "฿", "₩"
   };
   double valeur;
   string monnaie;

   public Prix(double valeur, string monnaie = "€")
   {
      if (monnaieValid.Contains(monnaie))
      {
         valeur = valeur;
         monnaie = monnaie;
      }
      else
      {
         throw new ArgumentException(String.Format("{0} is not a valid option", monnaie), "moannaie");
      }
   }
}

Something I want to add to the other answers. Normally in finance money is saved in cents, etc., i.e. an integer.

class Price {

    int cents;
    string currency;

    public Price(int cents, string currency)
    {
        this.cents = cents
        ...
    }

    ...

}

This fits better because we don't have 1e-308 € intervals. Moreover floating point arithmetic has its own problems which you can avoid when using ints.