Feat: Added persistance to the cl by darshan2006-op · Pull Request #13 · cyclohexane-2/cli

darshan2006-op · 2025-11-23T14:20:05Z

Implement Data Persistence #8

Data persistence has been implemented with all the requirements.

Copilot

Pull request overview

This PR implements data persistence for the student management CLI application, allowing assignments and grades to be saved and loaded from a JSON file.

Key Changes:

Added custom JSON encoder/decoder classes to handle datetime serialization
Implemented save/load functionality in StudentManager with error handling for missing or corrupted files
Integrated persistence into the main application flow to automatically load on startup and save on exit

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 10 comments.

File	Description
utils.py	Added DateTimeEncoder/DateTimeDecoder classes for datetime serialization and enhanced error handling in load_from_json
student_manager.py	Added persistence methods (dump_manager, load_manager) and serialization helpers (__ser_object, __deser_object)
main.py	Integrated data loading on startup and saving on exit with error handling for corrupted files
.gitignore	Added blank line (formatting change)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

main.py

student_manager.py

utils.py

main.py

student_manager.py

utils.py

Copilot

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-24T20:45:42Z

utils.py

+        return {
+            "status": 1
+        }
+    except json.JSONDecodeError as e:
+        os.remove(filename)
+        return {
+            "status": 2
+        }


The return value structure is inconsistent. When successful, the function returns the actual data dictionary, but on error it returns a status dictionary with a "status" key. This makes it difficult for callers to distinguish between actual data that might contain a "status" key and error conditions. Consider using a more explicit pattern like returning a tuple (data, status) or raising exceptions for error cases.

Copilot · 2025-11-24T20:45:42Z

utils.py

+        """Handles special case of date deserialization"""
+        for key, value in obj.items():
+            if isinstance(value, str):
+                try:
+                    obj[key] = datetime.fromisoformat(value)
+                except ValueError:
+                    pass  # Not a datetime string
+        return obj



The object_hook method iterates over dictionary items and attempts to modify the dictionary during iteration, which can work but only handles top-level datetime strings. Nested dictionaries or lists containing datetime strings won't be properly converted. Additionally, this approach will attempt to parse every string value as a datetime, which could be inefficient for large objects with many string fields.

Suggested change

"""Handles special case of date deserialization"""

for key, value in obj.items():

if isinstance(value, str):

try:

obj[key] = datetime.fromisoformat(value)

except ValueError:

pass # Not a datetime string

return obj

"""Handles special case of date deserialization, including nested structures"""

return self._decode_dates(obj)

def _decode_dates(self, value):

if isinstance(value, dict):

return {k: self._decode_dates(v) for k, v in value.items()}

elif isinstance(value, list):

return [self._decode_dates(item) for item in value]

elif isinstance(value, str):

try:

return datetime.fromisoformat(value)

except ValueError:

return value

else:

return value

Copilot · 2025-11-24T20:45:43Z

student_manager.py

+        if "status" in data:
+            status = data.get("status")
+            return cls(), status
+        return cls.__deser_object(data), 0


Missing cls parameter in the call to __deser_object. Should be cls._StudentManager__deser_object(data) due to name mangling, or better yet, change the method to use single underscore as noted in another comment.

Copilot · 2025-11-24T20:45:43Z

student_manager.py

+    def dump_manager(self):
+        """Dumps data into json file for persistence"""
+        data = self.__ser_object()
+        save_to_json(data, self.persistence_FILE_NAME)


Typo in method name: persistence_FILE_NAME should be PERSISTENCE_FILE_NAME to match the constant defined at line 10.

Suggested change

save_to_json(data, self.persistence_FILE_NAME)

save_to_json(data, self.PERSISTENCE_FILE_NAME)

Copilot · 2025-11-24T20:45:43Z

student_manager.py

+
+    def __ser_object(self):
+        """
+        Protected method to aggrigate data


Typo in comment: aggrigate should be aggregate.

Suggested change

Protected method to aggrigate data

Protected method to aggregate data

xenonnn4w · 2025-11-24T20:49:29Z

fix these, rest lgtm!

xenonnn4w · 2025-11-24T20:50:35Z

Also do squash your commits into one with a better commit message, Thanks!

xenonnn4w · 2025-12-11T13:52:53Z

Resolve the conflict with the main, i.e. rebase it to upstream. Also mark the conversation resolved if you are done with that.

Copilot

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

utils.py:1

Corrected spelling of 'aggrigate' to 'aggregate'.

"""

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

xenonnn4w · 2025-12-12T07:53:47Z

Lgtm thanks!

xenonnn4w requested a review from Copilot November 23, 2025 17:23

Copilot started reviewing on behalf of xenonnn4w November 23, 2025 17:24 View session

Copilot finished reviewing on behalf of xenonnn4w November 23, 2025 17:25

Copilot AI reviewed Nov 23, 2025

View reviewed changes

darshan2006-op force-pushed the add-feature-8 branch from 17f5ccc to e41a4f2 Compare November 24, 2025 12:35

xenonnn4w requested a review from Copilot November 24, 2025 20:42

Copilot started reviewing on behalf of xenonnn4w November 24, 2025 20:43 View session

Copilot finished reviewing on behalf of xenonnn4w November 24, 2025 20:44

Copilot AI reviewed Nov 24, 2025

View reviewed changes

xenonnn4w added the Need Author Reply label Dec 10, 2025

darshan2006-op force-pushed the add-feature-8 branch from 6edbb11 to e0fc1f0 Compare December 10, 2025 07:55

added data persistence

81eecfa

xenonnn4w requested a review from Copilot December 11, 2025 13:53

Copilot AI reviewed Dec 11, 2025

View reviewed changes

darshan2006-op force-pushed the add-feature-8 branch 2 times, most recently from bb3a203 to 81eecfa Compare December 11, 2025 17:09

xenonnn4w merged commit d2c399f into cyclohexane-2:main Dec 12, 2025

-        """Handles special case of date deserialization"""
-        for key, value in obj.items():
-            if isinstance(value, str):
-                try:
-                    obj[key] = datetime.fromisoformat(value)
-                except ValueError:
-                    pass  # Not a datetime string
-        return obj
+        """Handles special case of date deserialization, including nested structures"""
+        return self._decode_dates(obj)
+    def _decode_dates(self, value):
+        if isinstance(value, dict):
+            return {k: self._decode_dates(v) for k, v in value.items()}
+        elif isinstance(value, list):
+            return [self._decode_dates(item) for item in value]
+        elif isinstance(value, str):
+            try:
+                return datetime.fromisoformat(value)
+            except ValueError:
+                return value
+        else:
+            return value

	save_to_json(data, self.persistence_FILE_NAME)
	save_to_json(data, self.PERSISTENCE_FILE_NAME)

	Protected method to aggrigate data
	Protected method to aggregate data

Conversation

darshan2006-op commented Nov 23, 2025 • edited by xenonnn4w Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

xenonnn4w commented Nov 24, 2025

Uh oh!

xenonnn4w commented Nov 24, 2025

Uh oh!

xenonnn4w commented Dec 11, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

xenonnn4w commented Dec 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

darshan2006-op commented Nov 23, 2025 •

edited by xenonnn4w

Loading