3.4 KiB

Code explanation

This is a program manages the processing of orders in the following way: when an order is registered with the RegisterOrder method, the order is first deleted using the DeleteOrder method. Then, the order is converted to a list of ReviewExportLine objects using the ConvertOrderToExportLines method and added to a queue of lists of ReviewExportLine objects. There is a separate thread running in the background which was mentioend in the assignment itself. This service continuously checks the queue and processes the review export lines when the queue is not empty and the review folder is empty. When the review folder is not empty, the thread waits for 2 seconds and checks the queue again. The processing of review export lines involves opening a file in the review folder with a random 7 character string as the file name, writing the review export lines to the file, and then closing the file.

Problems with the code

  1. The ProcessReviewExportLines method runs in an infinite loop, which means that it will run indefinitely until the program is terminated. This may not be desirable behavior, especially if the program is intended to run for a fixed amount of time or in response to specific user input. Also, this seems hacky: a better practice would be to use some sort of a listener or a state observer (with observer pattern).

  2. Possible race-conditions: the ProcessReviewExportLines method checks whether the review folder is empty before processing the review export lines. However, it is possible that other processes or threads may be writing to the review folder at the same time, which could cause the program to skip processing the review export lines even if the folder is not actually empty.

  3. The ProcessReviewExportLines method writes to the review folder using a randomly generated file name. This may cause problems if the program needs to read the contents of the review folder at a later time, as it will be difficult to determine which file corresponds to which order.

  4. The FilterString method replaces semicolons (;) with colons (:). This may cause problems if the input strings contain colons that are not intended to be replaced. If it is such a simple operation, then it should be done locally.

  5. The DeleteOrder method enqueues a single ReviewExportLine object with a Type field value of "9" to the queue. It is not clear what the purpose of this line is, or how it is related to the process of deleting an order.

  6. GenerateRandomString method does not check the validity of the returned path: Since it generates these characters randomly, there could be a character that is considered illegal to be used as in a system-path. Additionally, 7 characters (which are very bounded) might not prove to be enough to prevent collisions.

How to solve them

  1. Add a condition to the loop to terminate it after a certain number of iterations or after a certain amount of time has passed Alternatively, a cancellation token to the method and pass it a token that can be used to cancel the loop. Or used better design-patterns

  2. Use a locking-mechanism to syncronize access to a shared resource, in this case, this folder.

  3. Use an incrementative / more predictable file name. Such as using timestamps.

  4. Move the operation upwards, or use the method better, to actually filter-out invalid characters.

  5. Add proper useful logic. Or add comments explaining it.