CODEWITHSUNDEEP
c#
Usher
Usher

Reputation: 2146

best way to handle conditional statement

I have the following condition using in my code but that doesn't look very efficient,is this any better way to handle ?

if (ic = filename.Contains(".wmv"))
{
    if (bitnumber > 400)
    {
        path = "ftp://" + ftpServerIP + "/" + "media" + "/" + "lib" + "/" + programName + "/" + date + "/";
        UploadCondition(path, filename);
        //return path;
    }
}

if (ic = filename.Contains(".wmv"))
{
    if (bitnumber < 400)
    {
        path = "ftp://" + ftpServerIP + "/" + "mpegmedia" + "/" + "news" + "/" + programName + "/" + "video" + "/" + "podcast" + "/";
        UploadCondition(path, filename);
        //return path;
    }
}

if (ic = filename.Contains(".m4v"))
{
    path = "ftp://" + ftpServerIP + "/" + "mpegmedia" + "/" + "news" + "/" + programName + "/" + "video" + "/" + "podcast" + "/";
    UploadCondition(path, filename);
}

if (ic = filename.Contains(".mp4"))
{
    path = "ftp://" + ftpServerIP + "/" + "mpegmedia" + "/" + "news" + "/" + programName + "/" + "video" + "/" + "podcast" + "/";
    UploadCondition(path, filename);
}
if (ic = filename.Contains(".flv"))
{
    path = "ftp://" + ftpServerIP + "/" + "mpegmedia" + "/" + "news" + "/" + programName + "/" + "video" + "/" + "podcast" + "/";
    UploadCondition(path, filename);
}
if (ic = filename.Contains(".mpg"))
{
    path = "ftp://" + ftpServerIP + "/" + "mpegmedia" + "/" + "news" + "/" + programName + "/" + "video" + "/" + "podcast" + "/";
    UploadCondition(path, filename);
}
if (ic = filename.Contains(".aac"))
{
    path = "ftp://" + ftpServerIP + "/" + "mpegmedia" + "/" + "news" + "/" + programName + "/" + "audio" + "/" + "podcast" + "/";
    UploadCondition(path, filename);
}
if (ic = filename.Contains(".mp3"))
{
    path = "ftp://" + ftpServerIP + "/" + "mpegmedia" + "/" + "news" + "/" + programName + "/" + "audio" + "/" + "podcast" + "/";
    UploadCondition(path, filename);
}

Upvotes: 1

Views: 159

Answers (7)

M.Babcock
M.Babcock

Reputation: 18965

Use a switch statement and System.IO.Path.GetExtension.

select (System.IO.Path.GetExtension(filename))
{
    case ".wmv":
        if (bitnumber > 400)
        {
            path = "ftp://" + ftpServerIP + "/" + "media" + "/" + "lib" + "/" + programName + "/" + date + "/";
            UploadCondition(path, filename);
            //return path;
        }
        else
        {
            path = "ftp://" + ftpServerIP + "/" + "mpegmedia" + "/" + "news" + "/" + programName + "/" + "video" + "/" + "podcast" + "/";
            UploadCondition(path, filename);
            //return path;
        }
        break;

        case ".m4v":
        case ".mp4":
        case ".flv":
        case ".mpg":
        case ".mp3":
        default:
            path = "ftp://" + ftpServerIP + "/" + "mpegmedia" + "/" + "news" + "/" + programName + "/" + "video" + "/" + "podcast" + "/";
            UploadCondition(path, filename);
            break;
    }
}

I'm guessing you'll want variations for the last block, but this should be easy enough to modify.

Upvotes: 1

nybbler
nybbler

Reputation: 4841

Something like this is a nice short solution to your problem and I believe it handles all of the cases your if statements are handling. 

String[] videoExtensions = { "wmv", "m4v", "mp4", "flv" };
String[] audioExtensions = { "aac", "mp3" };

String ext = Path.GetExtension(filename).ToLower();
String path = "ftp://" + ftpServerIP + "/";

if (-1 != Array.IndexOf(videoExtensions, ext)) {
  if ("wmv".equals(ext) && bitnumber > 400)
    path += "media/lib/" + programName + "/" + date + "/";
  else 
    path += "mpegmedia/news/" + programName + "/video/podcast/";
}
else if (-1 != Array.IndexOf(audioExtensions, ext)) {
  path += "mpegmedia/news/" + programName + "/audio/podcast/";
}​​​​​​​​​​​
else {
  // handle unknown extension types as desired
}

UploadCondition(path, filename);

Upvotes: 1

jgauffin
jgauffin

Reputation: 101150

Break it up in other classes like:

public class AudioFileValidator
{
    private List<string> _extensions = new List<string>{".aac", ".mp3"};
    public bool IsValid(string filename)
    {
        if (!_extensions.Contains(Path.GetExtension(filename))
            return false;

        //validate bitrate etc
    }
}

Usage:

var audioValidator = new AudioFileValidator();
if (audioValidator.IsValid(filename)) 
{
    path = "ftp://" + ftpServerIP + "/" + "mpegmedia" + "/" + "news" + "/" + programName + "/" + "audio" + "/" + "podcast" + "/";
    UploadCondition(path, filename);
}

var videoValidator = new VideoFileValidator();
if (videoValidator.IsValid(filename)) 
{
    path = "ftp://" + ftpServerIP + "/" + "mpegmedia" + "/" + "news" + "/" + programName + "/" + "video" + "/" + "podcast" + "/";
    UploadCondition(path, filename);
}

By doing so you'll get classes with a single responsibility which can be reused in other places and which are easy to unit test.

You could even take it further and introduce a new interface called IMediaFileValidator which all validators implement. and do something like:

foreach (var validator in validators)
{ 
    if (validator.IsValid(filename))
    {
        // use info from the validator to build the path
        var mediaName = validator.MediaName; 
        path = "ftp://" + ftpServerIP + "/" + mediaName + "/" + "news" + "/" + programName + "/" + "video" + "/" + "podcast" + "/";
        UploadCondition(path, filename);
        break;
    }
}

Which would also make your code adhere to Open/Closed principle.

Upvotes: 3

Fadrian Sudaman
Fadrian Sudaman

Reputation: 6465

You will need a lot of refactoring. Here are couple of ideas to get you started:

  • Use String.Format and passed in only value that changed to save you all the repeating the text
  • Build a dictionary of Extension/Ext-Combination key and set the value to the the destination path. You will then only require one lookup than big nesting if - else statements
  • Use Path.GetExtension rather than Contains to be more accurate

Eg. 

  string formatStringNews = "ftp://{0}/news/{1}/";
  string formatStringMedia = "ftp://{0}/media/{1}/";
  dictionary["wmv"] = formatStringMedia;
  dictionary["mp3"] = formatStringNews;
  ....
  string key = Path.GetExtension(filename);
  path = string.Format(dictionary[key], serverName, programName);

Upvotes: 2

V4Vendetta
V4Vendetta

Reputation: 38210

You could perhaps make it a bit simpler like 

if (filename.Contains(".wmv"))
    // path =  set the path as you require

and after all the ifs end call your method

UploadCondition(path, filename);

Better would be to extract the extension .wmv, .m4v from filename and make this into a switch whereby you set the path.

Upvotes: 0

Shashank Kadne
Shashank Kadne

Reputation: 8101

I assume, at a time your filename will be either be .m4v, .flv,.mp4 etc...so here goes the code..

    if (ic = filename.Contains(".wmv"))
{
    if (bitnumber > 400)
    {
        path = "ftp://" + ftpServerIP + "/" + "media" + "/" + "lib" + "/" + programName + "/" + date + "/";
        UploadCondition(path, filename);
        //return path;
    }

        else
        {
        path = "ftp://" + ftpServerIP + "/" + "mpegmedia" + "/" + "news" + "/" + programName + "/" + "video" + "/" + "podcast" + "/";
        UploadCondition(path, filename);
        //return path;
    }
}

    else if (ic = filename.Contains(".m4v"))
{
    path = "ftp://" + ftpServerIP + "/" + "mpegmedia" + "/" + "news" + "/" + programName + "/" + "video" + "/" + "podcast" + "/";
    UploadCondition(path, filename);
}

else if (ic = filename.Contains(".mp4"))
{
    path = "ftp://" + ftpServerIP + "/" + "mpegmedia" + "/" + "news" + "/" + programName + "/" + "video" + "/" + "podcast" + "/";
    UploadCondition(path, filename);
}
else if (ic = filename.Contains(".flv"))
{
    path = "ftp://" + ftpServerIP + "/" + "mpegmedia" + "/" + "news" + "/" + programName + "/" + "video" + "/" + "podcast" + "/";
    UploadCondition(path, filename);
}
else if (ic = filename.Contains(".mpg"))
{
    path = "ftp://" + ftpServerIP + "/" + "mpegmedia" + "/" + "news" + "/" + programName + "/" + "video" + "/" + "podcast" + "/";
    UploadCondition(path, filename);
}
else if (ic = filename.Contains(".aac"))
{
    path = "ftp://" + ftpServerIP + "/" + "mpegmedia" + "/" + "news" + "/" + programName + "/" + "audio" + "/" + "podcast" + "/";
    UploadCondition(path, filename);
}
else if (ic = filename.Contains(".mp3"))
{
    path = "ftp://" + ftpServerIP + "/" + "mpegmedia" + "/" + "news" + "/" + programName + "/" + "audio" + "/" + "podcast" + "/";
    UploadCondition(path, filename);
}
    else
    {
        //No Match found
    }

and the best approach would be to use Switch(fileExtn)

Upvotes: 0

EverPresent
EverPresent

Reputation: 1970

At the very least you could convert it into an if/elseif statement:

if (ic....)
{
    ...
} else if (ic...) {
    ...
}

Upvotes: 0

Related Questions