Calculating German common holidays

Question

I wrote a simple dll which is able to calculate all common holidays, but, I would like to optimize the code in readability and flexibility.

Federal state

public enum Bundesland
{
    BadenWürttemberg,
    Bayern,
    Berlin,
    Brandenburg,
    Bremen,
    Hamburg,
    Hessen,
    MecklenburgVorpommern,
    Niedersachsen,
    NordrheinWestfalen,
    RheinlandPfalz,
    Saarland,
    Sachsen,
    SachsenAnhalt,
    SchleswigHolstein,
    Thüringen
}

holiday

/// <summary>
/// Representiert den Feiertag mit
/// allen wichtigen informationen
/// </summary>
public class Feiertag : IComparable<Feiertag>
{
    private bool isFix;
    private DateTime datum;
    private string name;

    public Feiertag(bool isFix, DateTime datum, string name)
    {
        this.IsFix = isFix;
        this.Datum = datum;
        this.Name = name;
    }

    /// <summary>
    /// Der Name des Feiertages
    /// </summary>
    public string Name
    {
        get { return name; }
        set { name = value; }
    }

    /// <summary>
    /// Das Datum an dem dieser Feiertag stattfindet
    /// </summary>
    public DateTime Datum
    {
        get { return datum; }
        set { datum = value; }
    }

    /// <summary>
    /// Zeigt an ob es sich um einen Datums spezifischer
    /// oder zyklisch Feiertag handelt
    /// </summary>
    public bool IsFix
    {
        get { return isFix; }
        set { isFix = value; }
    }

    #region IComparable<Feiertag> Member

    public int CompareTo(Feiertag other)
    {
        return this.datum.Date.CompareTo(other.datum.Date);
    }

    #endregion IComparable<Feiertag> Member
}

`holidaylogic`:

/// <summary>
/// Hält eine Liste von Feiertagen für die Jahr Monat Kombination
/// </summary>
public class MyFeiertagLogic
{
    private static MyFeiertagLogic Instance;
    private List<Feiertag> feiertage;
    private int year;

    /// <summary>
    /// Das Jahr für welches die Feiertage berechnet werden
    /// </summary>
    public int CurrentYear
    {
        get { return year; }
        set { year = value; }
    }

    /// <summary>
    /// Erzeugt eine neue Instanz der Feiertage für das Übergebene Jahr außerdem wird davon ausgegangen das es sich bei dem Bundesland um Sachsen handelt
    /// </summary>
    /// <param name="year">Das Jahr für welches Die Feiertaglogic Initialisiert werden soll</param>
    /// <returns></returns>
    public static MyFeiertagLogic GetInstance(int year)
    {
        if (Instance == null || year != Instance.CurrentYear)
        {
            Instance = new MyFeiertagLogic(year, Bundesland.Sachsen);
            return Instance;
        }

        return Instance;
    }

    /// <summary>
    /// Erzeugt eine neue Instanz der Feiertage für das Übergebene Jahr und Bundesland
    /// </summary>
    /// <param name="year">Das Jahr für welches Die Feiertaglogic Initialisiert werden soll</param>
    /// <param name="bl">Das Bundesland welches zur Ermittlung der Feiertage betrachtet werden soll</param>
    /// <returns></returns>
    public static MyFeiertagLogic GetInstance(int year, Bundesland bl)
    {
        if (Instance == null || year != Instance.CurrentYear)
        {
            Instance = new MyFeiertagLogic(year, bl);
            return Instance;
        }

        return Instance;
    }

    /// <summary>
    /// Beschreibung: Gibt variable Feiertage zurueck
    /// </summary>
    public List<Feiertag> Feiertagliste
    {
        get { return feiertage; }
    }

    /// <summary>
    ///  prüft ob das übermittelte Datum ein Feiertag ist
    /// </summary>
    /// <param name="value">zu prüfendes Datum</param>
    /// <returns>True wenn ja</returns>
    public bool isFeiertag(DateTime value)
    {
        return (feiertage.Find(delegate(Feiertag f) { return f.Datum.Date == value.Date; }) != null);
    }

    /// <summary>
    /// gibt den Names des Feirtages zurück wenn das
    /// übergebene Datum ein Feiertag ist
    /// </summary>
    /// <param name="value">Feiertagsdatum</param>
    /// <returns>Name des Feiertages</returns>
    public Feiertag GetFeiertagName(DateTime value)
    {
        return (feiertage.Find(delegate(Feiertag f) { return f.Datum.Date == value.Date; }));
    }

    /// <summary>
    /// Beschreibung: gibt feste Feiertage zurueck
    /// </summary>
    public List<Feiertag> FesteFeiertage
    {
        get { return feiertage.FindAll(delegate(Feiertag f) { return f.IsFix; }); }
    }

    private MyFeiertagLogic(int year, Bundesland BL)
    {
        this.CurrentYear = year;

        #region fillList

        DateTime osterSonntag = GetOsterSonntag();
        DateTime bußuBettag = GetBußuBetTag();

        this.feiertage = new List<Feiertag>();

        //alle Bundesländer
        this.feiertage.Add(new Feiertag(true, new DateTime(year, 1, 1), "Neujahr"));

        if (BL == Bundesland.BadenWürttemberg || BL == Bundesland.Bayern || BL == Bundesland.SachsenAnhalt)
            this.feiertage.Add(new Feiertag(true, new DateTime(year, 1, 6), "Heilige Drei Könige"));

        //if(BL == Bundesland.BadenWürttemberg)
        //this.feiertage.Add(new Feiertag(false, osterSonntag.AddDays(-3), "Gründonnerstag"));

        //alle Bundesländer
        this.feiertage.Add(new Feiertag(false, osterSonntag.AddDays(-2), "Karfreitag"));

        if (BL == Bundesland.Brandenburg)
            this.feiertage.Add(new Feiertag(false, osterSonntag, "Ostersonntag"));

        //alle Bundesländer
        this.feiertage.Add(new Feiertag(false, osterSonntag.AddDays(1), "Ostermontag"));

        //alle Bundesländer
        this.feiertage.Add(new Feiertag(true, new DateTime(year, 5, 1), "Tag der Arbeit"));

        //alle Bundesländer
        this.feiertage.Add(new Feiertag(false, osterSonntag.AddDays(39), "Christi Himmelfahrt"));

        if (BL == Bundesland.Brandenburg)
            this.feiertage.Add(new Feiertag(false, osterSonntag.AddDays(49), "Pfingstsonntag"));

        //alle Bundesländer
        this.feiertage.Add(new Feiertag(false, osterSonntag.AddDays(50), "Pfingstmontag"));

        if (BL == Bundesland.BadenWürttemberg || BL == Bundesland.Bayern || BL == Bundesland.Hessen || BL == Bundesland.NordrheinWestfalen || BL == Bundesland.RheinlandPfalz || BL == Bundesland.Saarland)
            this.feiertage.Add(new Feiertag(false, osterSonntag.AddDays(60), "Fronleichnam"));

        if (/*BL == Bundesland.Bayern || */ BL == Bundesland.Saarland)
            this.feiertage.Add(new Feiertag(true, new DateTime(year, 8, 15), "Mariä Himmelfahrt"));

        //alle Bundesländer
        this.feiertage.Add(new Feiertag(true, new DateTime(year, 10, 3), "Tag der dt. Einheit"));

        if (BL == Bundesland.Brandenburg || BL == Bundesland.MecklenburgVorpommern || BL == Bundesland.Sachsen || BL == Bundesland.SachsenAnhalt || BL == Bundesland.Thüringen)
            this.feiertage.Add(new Feiertag(true, new DateTime(year, 10, 31), "Reformationstag"));

        if (BL == Bundesland.BadenWürttemberg || BL == Bundesland.Bayern || BL == Bundesland.NordrheinWestfalen || BL == Bundesland.RheinlandPfalz || BL == Bundesland.Saarland)
            this.feiertage.Add(new Feiertag(true, new DateTime(year, 11, 1), "Allerheiligen "));

        if (BL == Bundesland.Sachsen)
            this.feiertage.Add(new Feiertag(false, bußuBettag, "Buß- u. Bettag"));

        //alle Bundesländer
        this.feiertage.Add(new Feiertag(true, new DateTime(year, 12, 25), "1. Weihnachtstag"));

        //alle Bundesländer
        this.feiertage.Add(new Feiertag(true, new DateTime(year, 12, 26), "2. Weihnachtstag"));

        #endregion fillList
    }

    /// <summary>
    /// Berechnet für das CurrentYear den Ostersonntag
    /// </summary>
    /// <returns>Datum für Ostersonntag</returns>
    private DateTime GetOsterSonntag()
    {
        int g, h, c, j, l, i;

        g = this.year % 19;
        c = this.year / 100;
        h = ((c - (c / 4)) - (((8 * c) + 13) / 25) + (19 * g) + 15) % 30;
        i = h - (h / 28) * (1 - (29 / (h + 1)) * ((21 - g) / 11));
        j = (this.year + (this.year / 4) + i + 2 - c + (c / 4)) % 7;

        l = i - j;
        int month = (int)(3 + ((l + 40) / 44));
        int day = (int)(l + 28 - 31 * (month / 4));

        return new DateTime(this.year, month, day);
    }

    /// <summary>
    /// Berechnet für das CurrentYear den Buß- und Bettag
    /// </summary>
    /// <returns>Datum für Buß- und Bettag</returns>
    public DateTime GetBußuBetTag()
    {
        /// Buß- und Bettag ist immer der
        /// Mittwoch vor dem 23. November
        var nov = new DateTime(this.year, 11, 23);

        for (int i = -1; i < 10; i--)
        {
            var d = nov.AddDays(i);
            if (d.DayOfWeek == DayOfWeek.Wednesday)
                return d;
        }

        throw new IndexOutOfRangeException("BußuBetTag konnte nicht gefunden werden");
    }
}

Usage:

        var feiertage = MyFeiertagLogic.GetInstance(date.Year);
        var isfeiertag = feiertage.isFeiertag(date);

You might want to tidy up the "if" statements in MyFeiertagLogic(int year, Bundesland BL) function. You may want to replace that with a smart enum that returns a "new Feiertag()" for a given "BL". — Jegan, Aug 4, 2015 at 14:34

200_success · Accepted Answer · 2015-08-04 20:13:15Z

Singleton bug

Your singleton implementation is wrong:

public static MyFeiertagLogic GetInstance(int year)
{
    if (Instance == null || year != Instance.CurrentYear)
    {
        Instance = new MyFeiertagLogic(year, Bundesland.Sachsen);
        return Instance;
    }

    return Instance;
}

public static MyFeiertagLogic GetInstance(int year, Bundesland bl)
{
    if (Instance == null || year != Instance.CurrentYear)
    {
        Instance = new MyFeiertagLogic(year, bl);
        return Instance;
    }

    return Instance;
}

If I try to do…

MyFeiertagLogic feiertageInSachsenDiesesJahr = MyFeiertagLogic.GetInstance(2015);
MyFeiertagLogic feiertageInBrandenburgDiesesJahr = MyFeiertagLogic.GetInstance(2015, Bundesland.Brandenburg);

… I'll get the instance for Saxony instead of a second instance for Brandenburg as expected.

Library design

I think that requiring the year to be specified in the GetInstance() call is a poor design. The year is part of the DateTime arguments passed to isFeierTag(DateTime) (which should be called IsFeierTag(DateTime), by the way) and GetFeiertagName(DateTime). It should be your library's job to figure out how to satisfy those queries by loading the data for the appropriate year.

Enum

The Bundesländer all have standard two-letter abbreviations. I think the code would be less awkward if you used those abbreviations as enums instead of the full names.

Community · Accepted Answer · 2017-04-12 07:31:53Z

Class Feiertag

You should use autoimplemented properties here, because you aren't doing any validation in the setters of the properties. In addition you should make the setters private because all you need from outside of this class, is to access the getters.

Instead of IsFix I would suggest to reverse it and call it IsVariable to make it clear that the holiday isn't always at the same day and month.

The usage of regions is IMHO an anti pattern which should be avoided.

See also are-regions-an-antipattern-or-code-smell

Is there a good use for regions?

No. There was a legacy use: generated code. Still, code generation tools just have to use partial classes instead. If C# has regions support, it's mostly because this legacy use, and because now that too many people used regions in their code, it would be impossible to remove them without breaking existent codebases.

Think about it as about goto. The fact that the language or the IDE supports a feature doesn't mean that it should be used daily. StyleCop SA1124 rule is clear: you should not use regions. Never.

This being said your class should look like so (without changing the IsFix)

/// <summary>
/// Representiert den Feiertag mit
/// allen wichtigen informationen
/// </summary>
public class Feiertag : IComparable<Feiertag>
{

    public Feiertag(bool isFix, DateTime datum, string name)
    {
        this.IsFix = isFix;
        this.Datum = datum;
        this.Name = name;
    }

    /// <summary>
    /// Der Name des Feiertages
    /// </summary>
    public string Name { get; private set; }

    /// <summary>
    /// Das Datum an dem dieser Feiertag stattfindet
    /// </summary>
    public DateTime Datum { get; private set; }

    /// <summary>
    /// Zeigt an ob es sich um einen Datums spezifischer
    /// oder zyklisch Feiertag handelt
    /// </summary>
    public bool IsFix { get; private set; }

    public int CompareTo(Feiertag other)
    {
        return this.datum.Date.CompareTo(other.datum.Date);
    }

}

class MyFeiertagLogic

Here the private constructor

private MyFeiertagLogic(int year, Bundesland BL)

lets say it directly is kind of ugly. You should distinguish between getting holiday which are equal for every federal state and for single states only holidays.

Extract the creation of the equal holidays to a separate method, leaving the constructor only to deal with the remaining holidays.

Like so

private IEnumerable<Feiertag> GetAllgemeineFeiertage(int year)
{
    IList<Feiertag> feiertage = new List<Feiertag>();
    feiertage.Add(new Feiertag(true, new DateTime(year, 1, 1), "Neujahr"));
    feiertage.Add(new Feiertag(false, osterSonntag.AddDays(-2), "Karfreitag"));

    .......

    return feiertage;
}

resulting in the constructor like so

private MyFeiertagLogic(int year, Bundesland BL)
{
    this.CurrentYear = year;

    DateTime osterSonntag = GetOsterSonntag();
    DateTime bußuBettag = GetBußuBetTag();

    this.feiertage = new List<Feiertag>(GetAllgemeineFeiertage(year));

    if (BL == Bundesland.BadenWürttemberg || BL == Bundesland.Bayern || BL == Bundesland.SachsenAnhalt)
        this.feiertage.Add(new Feiertag(true, new DateTime(year, 1, 6), "Heilige Drei Könige"));

    if (BL == Bundesland.Brandenburg)
        this.feiertage.Add(new Feiertag(false, osterSonntag, "Ostersonntag"));

    if (BL == Bundesland.Brandenburg)
        this.feiertage.Add(new Feiertag(false, osterSonntag.AddDays(49), "Pfingstsonntag"));

    if (BL == Bundesland.BadenWürttemberg || BL == Bundesland.Bayern || BL == Bundesland.Hessen || BL == Bundesland.NordrheinWestfalen || BL == Bundesland.RheinlandPfalz || BL == Bundesland.Saarland)
        this.feiertage.Add(new Feiertag(false, osterSonntag.AddDays(60), "Fronleichnam"));

    if (BL == Bundesland.Saarland)
        this.feiertage.Add(new Feiertag(true, new DateTime(year, 8, 15), "Mariä Himmelfahrt"));

    if (BL == Bundesland.Brandenburg || BL == Bundesland.MecklenburgVorpommern || BL == Bundesland.Sachsen || BL == Bundesland.SachsenAnhalt || BL == Bundesland.Thüringen)
        this.feiertage.Add(new Feiertag(true, new DateTime(year, 10, 31), "Reformationstag"));

    if (BL == Bundesland.BadenWürttemberg || BL == Bundesland.Bayern || BL == Bundesland.NordrheinWestfalen || BL == Bundesland.RheinlandPfalz || BL == Bundesland.Saarland)
        this.feiertage.Add(new Feiertag(true, new DateTime(year, 11, 1), "Allerheiligen "));

    if (BL == Bundesland.Sachsen)
        this.feiertage.Add(new Feiertag(false, bußuBettag, "Buß- u. Bettag"));

}

Because the holidays won't change that often, you could extract the creation of the remaining state specific holidays in their own methods like

private IEnumerable<Feiertag> GetFeiertageVonBadenWürttemberg(int year)
{

}

and using the AddRange() method of the List<Feiertag> to add them all at once. This will result in a small code duplication for some of the holidays, but IMHO it is easier to maintain, because if a holiday will be "deleted" it will happen for single states most of the time.

The GetOsterSonntag() method could benefit from better variables name, but as the formula won't change and you for sure have found this on the internet, I would keep it like it is. Maybe a comment would be nice to state from where ( I guess wikipedia ) you got this piece of code.

Why is the GetBußuBetTag() method public ?

Let us take a closer look at it

public DateTime GetBußuBetTag()
{
    /// Buß- und Bettag ist immer der
    /// Mittwoch vor dem 23. November
    var nov = new DateTime(this.year, 11, 23);

    for (int i = -1; i < 10; i--)
    {
        var d = nov.AddDays(i);
        if (d.DayOfWeek == DayOfWeek.Wednesday)
            return d;
    }

    throw new IndexOutOfRangeException("BußuBetTag konnte nicht gefunden werden");
}

there is a problem with this loop. No, not directly a problem, but it is not good. What I mean is the condition i < 10. First this condition will always be true, because you are starting with i = -1 and decrementing i for each iteration.
Second the maximum you need to go back will be 7 days if the 23rd of november would be a wednesday.
What I don't see is how the IndexOutOfRange exception could ever be thrown, which by the way isn't the correct exception type for this kind of problem.

I would like to encourage you to always use braces {}. Not using them for single statement if's can lead to error prone code.

You should try to translate your code to english. This will help Bob the maintainer, who maybe isn't a german speaking person. to maintain your code. But hey, that is only my personal opinion. ;-)

Based on the naming guidelines methods should be named using PascalCase casing. So the isFeiertag() should be named IsFeiertag().

The method

 public Feiertag GetFeiertagName(DateTime value)

does imply something else than the name of it. If I would call a method like this I would expect to get the name of the holiday and not an instance of the Feiertag class.

Dead code like

     //if(BL == Bundesland.BadenWürttemberg)
     //this.feiertage.Add(new Feiertag(false, osterSonntag.AddDays(-3), "Gründonnerstag"));

should be deleted because it is only adding noise to the code and reduces readability.

Stack Exchange Network

Calculating German common holidays

Federal state

holiday

`holidaylogic`:

Usage:

2 Answers 2

Singleton bug

Library design

Enum

Your Answer

Not the answer you're looking for? Browse other questions tagged
c#
datetime
or ask your own question.

Hot Network Questions

Calculating German common holidays

Federal state

holiday

holidaylogic:

Usage:

2 Answers 2

Singleton bug

Library design

Enum

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged c#datetime or ask your own question.

Related

Hot Network Questions

`holidaylogic`:

Not the answer you're looking for? Browse other questions tagged
c#
datetime
or ask your own question.