C# Design Questions [closed]_问答_开发者_运维开发者技术经验分享

Closed. This question needs to be more focused. It is not currently accepting answers.

Want to improve this question? Update the question so it focuses on one problem only by editing this post.

Closed 4 years ago.

Improve this question

How to approach unit testing of private methods?

I have a class that loads Employee data into a database. Here is a sample:

public class EmployeeFacade   
{
    public Employees EmployeeRepository = new Employees();  
    public TaxDatas TaxRepository = new TaxDatas();  
    public Accounts AccountRepository = new Accounts();  
    //and so on for about 20 more repositories etc.  

    public bool LoadAllEmployeeData(Employee employee)  
    {   
        if (employee == null)
            throw new Exception("...");


        bool exists = EmployeeRepository.FetchExisting(emps.Id);
        if (!exists)
        {   
            EmployeeRepository.AddNew(); 
        }

        try
        {
            EmployeeRepository.Id = employee.Id;
            EmployeeRepository.Name = employee.EmployeeDetails.PersonalDetails.Active.Names.FirstName;
            EmployeeRepository.SomeOtherAttribute;
        }
        catch() {}

        try
        {
            emps.Save();
        }
        catch(){}   


        try
        {
            LoadorUpdateTaxData(employee.TaxData);
        }
        catch() {}

        try
        {
        LoadorUpdateAccountData(employee.AccountData);
        }
        catch() {}
        ... etc. for about 20 more other employee objects

    }  

    private bool LoadorUpdateTaxData(employeeId, TaxData taxData)
    {
        if (taxData == null)
           throw new Exception("...");

        ...same format as above but using AccountRepository 

    }

    private bool LoadorUpdateAccountData(employee.TaxData)
    {   
        ...same format as above but using TaxRepository 
    }
}

I am writing an application to take serialised objects(e.g. Employee above) and load the data to the database.

I have a few design question that I would like opinions on:

A - I am calling this class "EmployeeFacade" because I am (attempting?) to use the facade pattern. Is it good practace to name the pattern on the class name?

B - Is it good to call the concrete entities of my DAL layer classes "Repositories" e.g. "EmployeeRepository" ?

C - Is using the repositories in this way sensible or should I create a method on the repository itself to take, say, the Employee and then load the data from there e.g. EmployeeRepository.LoadAllEmployeeData(Employee employee)? I am aim for cohesive class and but this will requrie the repository to have knowledge of the Employee object which may not be good?

D - Is there any nice way around of not having to check if an object is null at the begining of each method?

E - I have a EmployeeRepository, TaxRepository, AccountRepository declared as public for unit testing purpose. These are really private enities but I need to be able to substitute these with stubs so that the won't write to my database(I overload the save() method to do nothing). Is there anyway around this or do I have to expose them?

F - How can I test the private methods - or is this done (something tells me it's not)?

G- "emps.Name = employee.EmployeeDetails.PersonalDe开发者_Go百科tails.Active.Names.FirstName;" this breaks the Law of Demeter but how do I adjust my objects to abide by the law?

A - I wouldn't call it XXXFacade, but something more meaningful (which may in fact mean you should call it XXXFacade)

B - I would call them XXXRepository

C - I don't really understand your model here - you're passing in an Employee object and assigning its values to the equivilent values in EmployeeRepository. The Repository shouldn't contain data fields - each instance of the repository does not represent a row in the database. The Repository is a way of getting data in and out of the database, by operating on collections of entities from the database (ie: Repository is the table, Entities are the rows). I would expect the Repository object to have a Save method which takes an Employee object as a parameter and it persists it to the database. As well as a Load method which takes an Id and returns and Employee:

Employee myEmployee = repository.Load(112345);
myEmployee.Name = "New Name";
repository.Save(myEmployee);

The Repository base class doesn't need to know about the specific implementation of the Employee class, through the use of generics and polymorphism. Take a look at Sh#rpArchitecture for a good example of this pattern.

D - yes, put that common logic in an abstract base class (Repository)

E - don't make them public if they should be private. If you need the use the logic of the repository in your unit tests to simulate fetching data, implement a common interface and then mock that interface out in your tests. You don't need to test that the repository returns the correct data since data is transient and inconsistent in reality. Better to fake it and test your behaviour does what you expect on precanned data from a mock repository.

F - Don't. Test behaviour not implementation.

G - I don't think this issue exists if you examine your architecture as described above.

How to approach unit testing of private methods?

You shouldn't write tests for private methods.

The only possible way of creating private methods is a refactorings of already tested public methods.

A - I am calling this class "EmployeeFacade" because I am (attempting?) to use the facade pattern. Is it good practace to name the pattern on the class name?

I don't think testing private methods a good idea; however, you can test "internal" classes, which are similar to private in the sense that external assemblies will not have access to them, by marking them as Internal Visible to your unit test project.

AssemblyInfo.cs -- [assembly: InternalsVisibleTo("YourClass.Tests")]

B - Is it good to call the concrete entities of my DAL layer classes "Repositories" e.g. "EmployeeRepository" ?

I do this frequently, I don't think there is anything wrong with it.

C - Is using the repositories in this way sensible or should I create a method on the repository itself to take, say, the Employee and then load the data from there e.g. EmployeeRepository.LoadAllEmployeeData(Employee employee)? I am aim for cohesive class and but this will requrie the repository to have knowledge of the Employee object which may not be good?

Unless I don't understand correctly, I would keep them seperate. I typically use my Repository classes as simply CRUD helpers, I would write a wrapper around the repository that exposes the functionality you need.

D - Is there any nice way around of not having to check if an object is null at the begining of each method?

If there is, I don't know it, I would just use ArgumentNullException()

E - I have a EmployeeRepository, TaxRepository, AccountRepository declared as public for unit testing purpose. These are really private enities but I need to be able to substitute these with stubs so that the won't write to my database(I overload the save() method to do nothing). Is there anyway around this or do I have to expose them?

See my answer for A, marking them as Internal and then setting InternalsVisible To your unit test assembly. See also MSDN.

F - How can I test the private methods - or is this done (something tells me it's not)?

I do not typically test private methods, and private classes that need to tested I mark as internal and use them in my test assembly.

A - I don't think its particularly bad to use the pattern name in the class name, though I honestly don't know how often it's done.

F - I think zerkms is right, you probably have to make them public, test them, then make them private when you're satisfied. Once their private, you could still test public methods that make use of the private methods to ensure they continue working.

As for your DAL and such, I would suggest looking into LINQ to SQL, available in .NET 3.0 and higher. It's a nice framework for handling the abstraction layer between your business logic and the database. Here are a few links to check out...

Quick Tutorial for LINQ to SQL in C# Part 1 of Scott Guthrie's blog

Scott Guthrie has a lot of good stuff on LINQ, if you're interested, you should check out more of his posts.

A - IMO, yes. It immediate remind you the pattern, and help you understand the code, and this is maybe one of the important practices in code writing - letting other people understand your code.

B - I prefer the xxDAO convention (Data Access Object).

C - I prefer "service oriented programming", meaning a service that "knows" to save an employee and not a "repository object" that mix between "model" and "control".

D - Maybe using Aspect, but I don't recommend it.

E - You can create an interface for those classed, and inject them from "outside" using setters (just like spring does), or get them from some kind of factory, In that way it will be easy for you to replace the classes with mock, and still leave the members "private".

F - I think that those methods should be extracted out side of the "load employee" and be self services. IMO, you should abstract the "employee data" objects (especially if you got 20 of them :-)). and write a simple service that know to load a "employee data object" of any kind.

Hope that I helped,
Shay

Your naming convention seems ok.
By calling concrete repositories you are tightly coupling the system. Pass them repo objects in constructor. Or use a DI/IOC container.
If repository is returning employee it will know of it. You might want the repo to know the contract for an employee class.
If you are getting null value for something, you should make sure provider code does not send down nulls.
You can achieve that by implementing dependency injection properly and using interfaces.
Standard unit testing frameworks will not give you that, you will need something like Moles. A sample is show on this post
Use inheritance more than composition if you can. But if the object model requires that, then you are helpless in my opinion.