0
\$\begingroup\$

The class WalkingData storages a "date" and a "walked distance". The class also read the stored data.

 public class WalkingData
    {
        public DateTime Date { get; set; }
        public int WalkedDistance { get; set; }
        private string _filePath = @"c:\Data\Json.txt";

        //Read Data from Json File
        public List<WalkingData> GetAll()
        {
            //If file does not exist returns an empty list
            if (!File.Exists(_filePath)) return new List<WalkingData>();

            string jsonData;

            //Read the existing Json file
            using (StreamReader readtext = new StreamReader(_filePath))
            {
                jsonData = readtext.ReadToEnd();
            }

            //Deserialize the Json and returs a list of WalkingData
            return JsonConvert.DeserializeObject<List<WalkingData>>(jsonData);            
        }

        //save an instance of  WalkingData in Json file
        public void Save()
        {
            List<WalkingData> lstExistingWalkingData = new List<WalkingData>();

            //if existing data, load it into lstExistingWalkingData
            if (File.Exists(_filePath))
                lstExistingWalkingData = GetAll();

            //Add the current instace into lstExistingWalkingData
            lstExistingWalkingData.Add(this);

            //Serialize lstExistingWalkingData
            string output = JsonConvert.SerializeObject(lstExistingWalkingData);

            //Save the Json file
            using (StreamWriter w = new StreamWriter(_filePath))
            {
                w.WriteLine(output);
            }
        }      
    }

After I applied the Single Responsibility Principle I have the new code that I would like to confirm if I applied the principle in a reasonable way:

//This class is located on a library called BOL and has a reference to DAL library
    public class WalkingData
    {
        public DateTime Date { get; set; }
        public int WalkedDistance { get; set; }
    }

    //This class is located on a library called BOL and has a reference to DAL library
    public class WalkingDataManager
    {        
        WalkingDataRepository walkingDataRepository = new WalkingDataRepository();

        public List<WalkingData> GetAll()
        {
            return walkingDataRepository.GetAll();
        }

        public void Save(WalkingData walkingData)
        {
            walkingDataRepository.Save(walkingData);
        }
    }

    //this class is located in library Called DAL
    internal class WalkingDataRepository
    {
        private string _filePath = @"c:\Data\Json.txt";

        //Read Data from Json File
        internal List<WalkingData> GetAll()
        {
            //If file does not exist returns an empty list
            if (!File.Exists(_filePath)) return new List<WalkingData>();

            string jsonData;

            //Read the existing Json file
            using (StreamReader readtext = new StreamReader(_filePath))
            {
                jsonData = readtext.ReadToEnd();
            }

            //Deserialize the Json and returs a list of WalkingData
            return JsonConvert.DeserializeObject<List<WalkingData>>(jsonData);
        }

        //save an instance of  WalkingData in Json file
        internal void Save(WalkingData walkingData)
        {
            List<WalkingData> lstExistingWalkingData = new List<WalkingData>();

            //if existing data, load it into lstExistingWalkingData
            if (File.Exists(_filePath))
                lstExistingWalkingData = GetAll();

            //Add the current instace into lstExistingWalkingData
            lstExistingWalkingData.Add(walkingData);

            //Serialize lstExistingWalkingData
            string output = JsonConvert.SerializeObject(lstExistingWalkingData);

            //Save the Json file
            using (StreamWriter w = new StreamWriter(_filePath))
            {
                w.WriteLine(output);
            }
        }
    }

\$\endgroup\$
8
  • 1
    \$\begingroup\$ Welcome to code review, where we review working code from one of your projects and provide suggestions on how that code can be improved. Unfortunately we can't help you re-write / refactor code. If you write a solution that separates the CRUD from the rest of the class we will be happy to review that code and tell you what you might improve. \$\endgroup\$
    – pacmaninbw
    Apr 26, 2020 at 20:55
  • \$\begingroup\$ I made the re-factorization, can you please confirm if the new code has applied the SRP? \$\endgroup\$
    – Aben
    Apr 26, 2020 at 22:37
  • 2
    \$\begingroup\$ The current question title of your question is too generic to be helpful. Please edit to the site standard, which is for the title to simply state the task accomplished by the code. Please see How do I ask a good question?. \$\endgroup\$
    – BCdotWEB
    Apr 27, 2020 at 8:12
  • 1
    \$\begingroup\$ @pacmaninbw I think this basically boils down to a request for review of the second piece of code. The validation part is a bit off, but I imagine one of the answers could identify whether this code can be improved SRP-wise or not. \$\endgroup\$
    – Mast
    Apr 27, 2020 at 11:21
  • 1
    \$\begingroup\$ "classes located on some library": If this is code not under your control and included for completeness, only, please a) properly attribute (hyperlink for reference welcome) b) present as a block quote. \$\endgroup\$
    – greybeard
    Apr 28, 2020 at 5:34

1 Answer 1

1
\$\begingroup\$

The 3 classes now follow the SRP.

You may want to allow the file path to be updated by a parameter passed into the WalkingDataRepository constructor. This would allow a user to set the file path by command line arguments, question and answer at runtime or environment variable.

\$\endgroup\$

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.