1
\$\begingroup\$
using System;
using System.Collections.Generic;
using System.IO;
using WMPLib;
using System.Threading;

namespace MusicPlayer
{
    public class Player
    {
        public static List<string> unplayedSongs = new List<string>();
        public static List<string> playedSongs = new List<string>();
        public static WindowsMediaPlayer currentSong = new WindowsMediaPlayer();

        private static Thread songthread;

        private static Random rand = new Random();

        private static int count = 0;
        private static int volume = 1;
        private static bool repeat = true;

        static void Main(string[] args)
        {
            AddSongPaths();
            AddExcludePaths();

            Console.WriteLine("Would you like song repeats? (Y/N)");
            var input = Console.ReadLine();

            if (input.ToLower() == "n")
                repeat = false;

            PlaySong();

            Console.ReadLine();
        }

        private static void PlaySong()
        {
            play:
            songthread?.Abort();
            songthread = new Thread(() => PlaySong(unplayedSongs[SongLocation()])) {Name = $"PlaySongThread" };
            songthread.Start();

            loop:
            CleanAndDisplayName();

            while (true)
            {
                var input = Console.ReadLine();

                if (input == "skip" || input == "Skip")
                    goto play;

                if (input == "mute" || input == "mute")
                {
                    currentSong.settings.mute = !currentSong.settings.mute;
                    goto loop;
                }

                if (input == "pause" || input == "pause")
                {
                    currentSong.controls.pause();
                    goto loop;
                }

                if (input == "play" || input == "Play")
                {
                    currentSong.controls.play();
                    goto loop;
                }

                int s = int.MinValue;
                int.TryParse(input, out s);

                if (s != 0)
                {
                    volume = currentSong.settings.volume = int.Parse(input);
                    goto loop;
                }
            }
        }

        private static void CleanAndDisplayName()
        {
            Console.Clear();

            PrintControls();
            Console.WriteLine($"Currently Playing: {currentSong.currentMedia?.name}");

            if (currentSong.currentMedia == null)
                CleanAndDisplayName();
        }

        private static void PrintControls()
        {
            var controlls = "===========\nCommands:\nPause\nPlay\nSkip\nMute\n===========\nType a number for Volume Control: 1 - 10\n===========";
            Console.WriteLine(controlls);
        }

        private static void AddExcludePaths()
        {
            Console.WriteLine("Enter Paths to exclude (\"none\" to exclude no paths): ");
            List<string> exclude = new List<string>();

            while (true)
            {
                var input = Console.ReadLine();

                if (input == "none" || input == "done")
                    break;

                exclude.Add(input);
            }

            for (int i = 0; i < exclude.Count; i++)
            {
                for (int j = unplayedSongs.Count - 1; j >= 0; j--)
                {
                    if (unplayedSongs[j].Contains(exclude[i]))
                        unplayedSongs.RemoveAt(j);
                }
            }
        }

        public static void AddSongPaths()
        {
            Console.WriteLine("Enter Music Folder Path: ");

            enterMusicPath:
            string filePath = Console.ReadLine();

            if (!Directory.Exists(filePath))
            {
                Console.WriteLine("Please Enter a valid file path");
                goto enterMusicPath;
            }

            string[] paths = Directory.GetFiles(filePath, "*.*", SearchOption.AllDirectories);

            for (int i = 0; i < paths.Length; i++)
            {
                if (paths[i] == null)
                    return;

                if (paths[i].EndsWith(".mp3") || paths[i].EndsWith(".mp4") || paths[i].EndsWith("m4a"))
                    unplayedSongs.Add(paths[i]);
            }
        }

        public static void PlaySong(string path)
        {
            currentSong.URL = path;
            currentSong.settings.volume = 1;
            currentSong.controls.play();

            while (true)
            {
                Thread.Sleep(2000);

                if((currentSong.controls.currentPosition >= currentSong.currentMedia.duration) || (currentSong.controls.currentPosition <= 0.5))
                {
                    if(repeat)
                    {
                        PlaySongWithRepeats();
                    }
                    else
                    {
                        PlaySongWithoutRepeats();
                    }
                }
            }
        }

        public static void PlaySongWithRepeats()
        {
            currentSong.URL = unplayedSongs[SongLocation()];
            currentSong.settings.volume = volume;
            currentSong.controls.play();
            CleanAndDisplayName();
        }

        public static void PlaySongWithoutRepeats()
        {
            if(unplayedSongs.Count == 0)
            {
                songthread.Abort();
                Environment.Exit(exitCode: 0);
            }

            var location = SongLocation();

            currentSong.URL = unplayedSongs[location];
            currentSong.settings.volume = volume;
            currentSong.controls.play();
            CleanAndDisplayName();

            playedSongs.Add(unplayedSongs[location]);
            unplayedSongs.RemoveAt(location);
        }

        public static int SongLocation()
        {
            var location = rand.Next(0, unplayedSongs.Count);
            return (location == unplayedSongs.Count ? location - 1 : location);
        }
    }
}

This is my simple music shuffle program that i made using C# and WMPLib. What do you think? This is my first attempt at using threads for anything so if their is something that I did wrong or could do better please tell me. Also is their a better way to find files that are actually music files and not apposed to adding options of file extensions.

Github Link: MusicPlayer

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

You have a lot of bad things in your code.


How should I exit from your program? Alt+F4? You run endless loop without any break but with multiple goto statements that return me to endless loop again. You shoud avoid goto since it makes hard to understand your program flow.


What is the purpose of declaring some fields as public if you use them inside your class only?


From this code

if (input == "skip" || input == "Skip")

I make conclusion that "sKip" is invalid. If you want to compare string on equality ignoring case you should use string.Equals:

string.Equals(input, "skip", StringComparison.InvariantCultureIgnoreCase)

int s = int.MinValue;
int.TryParse(input, out s);

if (s != 0)
{
    volume = currentSong.settings.volume = int.Parse(input);
    goto loop;
}

Idea of TryParse method is to check whether parsing was successfull or not without an exception. You are ignoring result checking s on non-zero, but zero is a valid number that will be returned if parsing was failed. Also inside the if you are parsing the same string again. Correct code is:

int s;
if (int.TryParse(input, out s))
{
    volume = currentSong.settings.volume = s;
    goto loop;
}

This code

for (int i = 0; i < exclude.Count; i++)
{
    for (int j = unplayedSongs.Count - 1; j >= 0; j--)
    {
        if (unplayedSongs[j].Contains(exclude[i]))
            unplayedSongs.RemoveAt(j);
    }
}

can be simplified to

unplayedSongs.RemoveAll(s => exclude.Any(e => s.Contains(e)));

Why instead of this code

Console.WriteLine("Enter Music Folder Path: ");

enterMusicPath:
string filePath = Console.ReadLine();

if (!Directory.Exists(filePath))
{
    Console.WriteLine("Please Enter a valid file path");
    goto enterMusicPath;
}

don't use this:

string filePath; 

do
{
    Console.WriteLine("Please enter a valid music folder path:");
    filePath = Console.ReadLine();
}
while (!Directory.Exists(filePath));

var location = rand.Next(0, unplayedSongs.Count);
return (location == unplayedSongs.Count ? location - 1 : location);

If you read about Random.Next on MSDN, you'll see that it returns value that is greater than or equal to minValue and less than maxValue which means you'll never get location == unplayedSongs.Count.


Here

for (int i = 0; i < paths.Length; i++)
{
    if (paths[i] == null)
        return;

    if (paths[i].EndsWith(".mp3") || paths[i].EndsWith(".mp4") || paths[i].EndsWith("m4a"))
        unplayedSongs.Add(paths[i]);
}

you use index only to access an element so you don't need for loop. Use foreach:

foreach (var path in paths)
{
    if (string.IsNullOrEmpty(path))
        return;

    if (path.EndsWith(".mp3") || path.EndsWith(".mp4") || path.EndsWith("m4a"))
        unplayedSongs.Add(path);
}
\$\endgroup\$
2
  • \$\begingroup\$ Just wondering how does: unplayedSongs.RemoveAll(s => exclude.Any(e => s.Contains(e))); work? just haven seen it before \$\endgroup\$ Aug 12, 2017 at 17:09
  • \$\begingroup\$ RemoveAll method of the List<T> removes all elements that satisfy the specified condition. In this case condition is contains some string from exclude collection. Read about LINQ, it is very powerful feature of C# so you definitely should learn it. \$\endgroup\$
    – Maxim
    Aug 13, 2017 at 1:31

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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