# Is this method a good candidate for a service layer?

Should the following GetPhysicalFileLocation() method in the Car class have its own Service Layer?

public class Car
{
public int id;
public string model;
public int year;
public IList<Pic> lstCarPics;
}


Then we have the respective class:

public class Pic
{
public string filename;
public string fileLocation;
public string GetPhysicalFileLocation()
{
string LocalDirectory = "~\\CarFiles\\";
DirectoryInfo diPath = new
DirectoryInfo(HttpContext.Current.Server
.MapPath(LocalDirectory + car.id);
}
}


Seeing that this method and others will access physical file locations throughout the application, would something like this justify having it's own FileService?

I'm having trouble understanding when methods like this should fall under it's own service layer vs leaving it in its class.

-

As @Vogel612 hinted, a class shouldn't expose its fields publicly - this:

public class Car
{
public int Id { get; set; }

// ...
}


Is equivalent to this:

public class Car
{
private int _id;
public int Id
{
get { return _id; }
set { _id = value; }
}

// ...
}


As you can see, encapsulation is easily achieved with auto-properties - don't expose your fields!

Method GetPhysicalFileLocation has dependencies on the file system and on an HttpContext, so you definitely want that to be abstracted away, so you can write a test that uses that code and doesn't actually hit the file system.

I'd move it to some IFileSystemService implementation:

public interface IFileSystemService
{
string GetPhysicalFileLocation(int carId);

// ...
}


And then make a class that implements it:

public class FileSystemService : IFileSystemService
{
// load this value from application settings?
private readonly string _localDirectory = "~\\CarFiles\\";

public string GetPhysicalFileLocation(int carId)
{
var diPath = new DirectoryInfo(HttpContext.Current
.Server.MapPath(_localDirectory + carId);
}
}


And now you can have client code like this:

public class MyClass
{
private readonly IFileSystemService _service;

public MyClass(IFileSystemService service)
{
_service = service;
}

public void DoSomething(int carId)
{
var fileName = _service.GetPhysicalFileLocation(carId);
// ...
}
}


This way you can inject a "fake"/"mock" implementation for IFileSystemService and test DoSomething without hitting the file system or depending on an HttpContext; the key being that depending on the file system and on an HttpContext, is an implementation detail that the client code doesn't need to know about.

-
Ok. How would this get "glued" back up? So then would I have a GetPhysicalFileLocation method within the Pic class that calls the IFileService? –  TheTruthIsInCode Apr 25 '14 at 15:56
I'd make a separate class to implement the interface, and have whoever needs to GetPhysicalFileLocation call the interface's method (which would take a fileName parameter). –  Mat's Mug Apr 25 '14 at 15:59
Ok I'm lost in the code-woods now :( Can you create a quick sample using my code above how this would work? That's the part I'm not getting. Thanks! –  TheTruthIsInCode Apr 25 '14 at 16:11
@TheTruthIsInCode sorry that was confusing - edited with [hopefully] less confusing details ;) –  Mat's Mug Apr 25 '14 at 17:46
Hyper Clear!!! Thanks Mat!! –  TheTruthIsInCode Apr 25 '14 at 18:28

The convention for public fields and properties in c# is PascalCasing. This would make the following:

public class Car
{
public int Id; //{ get; set; } this makes it a property.
public string Model;
public int Year;
public IList<Pic> LstCarPics;
}

public class Pic
{
public string FileName;
public string FileLocation;
//...
}


Furthermore there is (from the information you posted) no real necessity to shorten the classname to Pic. Why not just use Picture?

-