1
\$\begingroup\$

I recently wrote my first XSD Schema:

<?xml version="1.0" encoding="UTF-8" ?>
<xs:schema xmlns:xs="http://www.w3.org/2001/XMLSchema">
<xs:element name="mediums">
    <xs:complexType>
        <xs:element name="medium" maxOccurs="unbounded">
            <xs:complexType>
                <xs:sequence>
                    <xs:element name="name" type="xs:token" />
                    <xs:element name="releaseDate" type="xs:date" />
                    <xs:element name="mediumType">
                        <xs:simpleType>
                            <xs:restriction base="xs:string">
                                <xs:enumeration value="CD"/>
                                <xs:enumeration value="DVD"/>
                            </xs:restriction>
                        </xs:simpleType>
                    </xs:element>
                    <xs:element name="length" type="length">
                    </xs:element>
                    <xs:element name="attributes">
                        <xs:complexType>
                            <xs:choice>
                                <xs:element name="actor" maxOccurs="unbounded">
                                    <xs:complexType>
                                        <xs:element name="name" type="xs:token"/>
                                        <xs:element name="age" type="xs:positiveInteger"/>
                                    </xs:complexType>
                                </xs:element>
                                <xs:element name="track" maxOccurs="unbounded">
                                    <xs:complexType>
                                        <xs:element name="name" type="xs:token"/>
                                        <xs:element name="artist" type="xs:token"/>
                                        <xs:element name="length" type="length"/>
                                    </xs:complexType>
                                </xs:element>
                            </xs:choice>    
                        </xs:complexType>
                    </xs:element>
                </xs:sequence>
                <xs:attribute name="id" type="xs:positiveInteger"    use="required" />
            </xs:complexType>
        </xs:element>
    </xs:complexType>
    </xs:element>

<!-- Represents a minute or a second value !-->
        <xs:simpleType name="max60Integer">
            <xs:restriction base="positiveInteger">
                <xs:maxExclusive value="60" />
            </xs:restriction>
        </xs:simpleType>
        <xs:complexType name="length">
            <xs:element name="hours" type="xs:positiveInteger"/>
            <xs:element name="minutes" type="max60Integer"/>
            <xs:element name="seconds" type="max60Integer"/>
        </xs:complexType>

It should represent following classes:

public enum MediumType {DVD, CD}

sealed class Medium : IEquatable<Medium>
{
    public static bool operator ==(Medium medium1,Medium medium2)
    {
        if ((object)medium1 == null||(object)medium2 == null)
        {
            return Object.Equals(medium1, medium2);
        }           
        return medium1.Equals(medium2) ? true : false;
    }
    public static bool operator !=(Medium medium1,Medium medium2)
    {
        if ((Object)medium1 == null || (Object)medium2 == null) return !Object.Equals(medium1, medium2);
        return !medium1.Equals(medium2);

    }
    public bool Equals(Medium medium)
    {
        if (medium == null) return false;
        if (this.Name != medium.Name)
            return false;
        if (this.Length != medium.Length)
            return false;
        if (this.ReleaseTime != medium.ReleaseTime)
            return false;
        return new HashSet<IAttribute>(Attributes).SetEquals(medium.Attributes);

    }
    public override bool Equals(object obj)
    {
        if (obj == null) return false;
        Medium medium = obj as Medium;
        if (medium == null) return false;
        else return Equals(medium);
    }

    public override int GetHashCode()
    {
        return Name.GetHashCode();
    }
    public override string ToString()
    {
        string classtype = Enum.GetName(typeof(MediumType),CurrentMediumType);
        return string.Format("{0} Name={1} Length={2} ReleaseTime={3}",classtype,Name,Length,ReleaseTime.Month+"."+ReleaseTime.Year);
    }
    public IAttribute this[int index]
    {
        get
        {
            return Attributes[index];
        }
            set
        {
            Attributes[index] = value;
        }
    }

    public Medium(string name,int length,DateTime releaseTime,MediumType mediumtype,long id)
    {
        this.Name = name;
        this.Length = length;
        this.ReleaseTime = releaseTime;
        this._mediumType = mediumtype;
        this.Attributes = new List<IAttribute>();
        ID = id;
    }
    public long ID { get; }
    private MediumType _mediumType;

    public MediumType CurrentMediumType
    {
        get { return _mediumType; }
    }

    public List<IAttribute> Attributes { get; set; }



    private string _name;

    public string Name
    {
        get { return _name; }
        set { if(value != null && value != "")
            {
                _name = value;
            }
        else
            {
                throw new Exception("A Medium must have a name!");
            }
        }
    }

    private int _length;

    public int Length
    {
        get { return _length; }
        set {
            if (value < 0)
                throw new Exception("The length of the film must be over 0!");
            else
                _length = value;
        }
    }

    private DateTime _releaseTime;

    public DateTime ReleaseTime
    {
        get { return _releaseTime; }
        set
        {
            if (DateTime.Compare(value,DateTime.Now) <= 0)
            {
                _releaseTime = value;
            }
else
            {
                throw new Exception("The release time can not be in the  future!");
            }
        }
    }



}

//Actor
public class Actor : IAttribute,IEquatable<Actor>
{
    public override int GetHashCode()
    {
        return Name.GetHashCode();
    }
    public override bool Equals(object obj)
    {
        if (obj == null) return false;
        Actor actor1 = obj as Actor;
        if (actor1 == null) return false;
        return Equals(actor1);

    }
    public static bool operator ==(Actor actor1,Actor actor2)
    {
        if (actor1 == null || actor2 == null) return   Object.Equals(actor1, actor2);
        return actor1.Equals(actor2);
    }
    public static bool operator !=(Actor actor1,Actor actor2)
    {
        return actor1 == actor2 ? false : true;
    }
    public bool Equals(Actor other)
    {
        if (other == null) return false;
        if (this.Name != other.Name) return false;
        if (this.Age != other.Age) return false;
        return true;
    }

    private string _name;

    public string Name
    {
        get { return _name; }
        set
        {
            if (value != null && value != "")
                _name = value;
            else
                throw new Exception("An actor needs an name!");
        }
    }
    private int _age;

    public int Age
    {
        get { return _age; }
        set
        {
            if (value < 0)
                throw new Exception("A actor can't be jounger than 0!");
            else if (value > 100)
                throw new Exception("The actor isn't older than 100, is he?");
            else
                _age = value;
        }
    }

    public Actor(string name,int age)
        {
        this.Name = name;
        this.Age = age;
    }

public override string ToString()
    {
        return string.Format("Actor Name={0} Age={1}",Name,Age);
    }



}
//Track
 public class Track : IAttribute,IEquatable<Track>
{
    public override int GetHashCode()
    {
        return Name.GetHashCode();
    }
    public static bool operator ==(Track track1,Track track2)
    {
        if (track1 == null || track2 == null)
            return Object.Equals(track1, track2);
        return track1.Equals(track2);
    }
    public static bool operator !=(Track track1,Track track2)
    {
        return  track1 == track2 ? false : true ;
    }
        public override bool Equals(object obj)
    {
        if (obj == null) return false;
        IAttribute attribut = obj as IAttribute;
        if (attribut == null) return false;
        return Equals(attribut);
    }
    public bool Equals(Track other)
    {
        if (other == null) return false;
        if (other.Name != this.Name) return false;
        if (other.Artist != this.Artist) return false;
        if (other.Length != this.Length) return false;
        return true;

    }
    private string _name;

    public string Name
    {
        get { return _name; }
        set
        {
            if (value != null && value != "")
                _name = value;
            else
                throw new Exception("An track needs a name!");
        }
    }

    private decimal _length;

    public decimal Length
    {
        get { return _length; }
        set
        {
            if (value > 0)
                _length = value;
            else
                throw new Exception("The length can not be under 0 minutes.");

        }
    }
    private string _artist;

    public string Artist
    {
        get { return _artist; }
        set
        {
            if (value != null && value != "")
                _artist = value;
            else
                throw new Exception("A artist needs a name!");
        }
    }

    public long ID { get;  }

    public Track(string name,decimal length,string artist)
    {
        this.Name = name;
        this.Length = length;
        this.Artist = artist;

    }
    public override string ToString()
    {
        return string.Format("Track Name={0} Artist={1} Length={2}",Name,Artist,Length);
    }
}

I will have look at target namespace etc. For now it's just about the design of my schema.

\$\endgroup\$
4
  • \$\begingroup\$ So what do you want to be reviewed? The xsd schema, the C# model, or both? \$\endgroup\$ Commented Apr 18, 2016 at 11:58
  • \$\begingroup\$ The schema ;) The Code ist just to show you, what i tried to represent in the schema. Hope you understand what i mean. \$\endgroup\$
    – user103180
    Commented Apr 18, 2016 at 12:12
  • \$\begingroup\$ "It should represent the following classes" - does it? You can use xsd.exe to generate classes from your schema. \$\endgroup\$ Commented Apr 18, 2016 at 13:57
  • \$\begingroup\$ Okay. Is the quality the same, as in self coded schemas? I made bad experiences with generated things, but i will try it... \$\endgroup\$
    – user103180
    Commented Apr 18, 2016 at 15:07

1 Answer 1

2
\$\begingroup\$

I have to admit it's been (thankfully) a while since I worked with XSD.

A few random points:

  • I would rename your max60Integer to "max59integer", since the maximum of 60 is excluded;

  • Age is defined as a positive integer in your schema: <xs:element name="age" type="xs:positiveInteger"/ - but your C# code allows it to be 0 (less restrictive than the schema), while disallowing it to exceed 100 (more restrictive than the schema). This is quite inconsistent.

  • This is cosmetics, but I would close empty tags.

    Eg.

    <xs:element name="length" type="length">
    </xs:element>
    

    should become

    <xs:element name="length" type="length" />
    

Purely as a side note - since you didn't mean to submit your C# code for a review:

public override bool Equals(object obj)
{
    if (obj == null) return false;
    Actor actor1 = obj as Actor;
    if (actor1 == null) return false;
    return Equals(actor1);
} 

I know you've only posted it as a reference point, but I have to say I find it very unreadable.

return clauses on the same line as the condition aren't considered good practice, plus the first if is totally redundant: if obj is null, then obviously obj as Actor will return null anyway. I'm also not sure what the ordinal is supposed to achieve. This method could be simplified to something way clearer imho:

public override bool Equals(object obj)
{
    Actor actor = obj as Actor;
    return actor != null && Equals(actor);
} 
\$\endgroup\$
1
  • \$\begingroup\$ Thank you, i will fix it. Forgot that as can handle null values... \$\endgroup\$
    – user103180
    Commented Apr 18, 2016 at 15:08

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.