[UserStory] Refactor "Play" function implementation

[nicholasmatthews-dev]

User Story

Essential components

• [x] Title describes the story
• [x] Stakeholder type is identified
• [x] Outcome is described
• [x] Rationale is explicit
• [x] Acceptance criteria are verifiable and from the perspective of the stakeholder

Story

As a Developer I want Audio functions to be separated so that the program is more maintainable and easily extendible

Acceptance Criteria

• Audio functions are encapsulated in an audio related class or classes
• Current play functionality remains unchanged
  • Clicking on "File>Play" for a given image will still result in the same audio for the user

Supporting Information

Currently, the play method is included inside of the imgProvider class. The imgProvider provides image information to generate the audio. Play method seems to be unrelated to the function of imgpProvider. Moving the play method to another class will allow further development of audio features and potentially allow for more extensibility of play methods.

Dependencies

Depends On

Dependents

[jody]

Refactoring for maintainability is highly desirable! Thanks for bringing attention to this.

Some concerns about the Acceptance Criteria:

• The acceptance criteria contain design constraints that are too specific. In particular, why is the maintainer restricted to a single class and why must that class be named audioProvider? (Side note: "audioProvider" violates our convention for class naming.)
• What is meant by "the current play method can be used"? Used by whom? Is this an instruction that the current source code lines can be used in the new class? That the current play method should be a dispatch to a method in the new class? Something else?
• There is no "play button" in the current application.
• What does "Audio can use current image to generate music" mean? What is the alternative?

[nicholasmatthews-dev]

Responding to the concerns about the acceptance criteria.

The acceptance criteria contain design constraints that are too specific. In particular, why is the maintainer restricted to a single class and why must that class be named audioProvider? (Side note: "audioProvider" violates our convention for class naming.)

This is certainly a valid concern, the original acceptance criteria were overly specific and mentioned a specific proposed solution. Likely the acceptance criteria should be more along the lines of, audio functions are inside of their own classes (should more classes be needed), and that is more agnostic with regards to the implementation. Likewise, the mention that the class name should be audioProvider is overly specific and should be omitted from the acceptance criteria.

What is meant by "the current play method can be used"? Used by whom? Is this an instruction that the current source code lines can be used in the new class? That the current play method should be a dispatch to a method in the new class? Something else?

I believe the intent is that any refactoring shouldn't remove functionality that is in the current version. Currently, when the user selects the play option under the file menu button "File>Play," a thread is opened to play audio which is generated from the ImgProvider instance that is open in the window where "File>Play" was selected, this is done by calling ImgProvider.play(). It may be necessary to change how the play method is called in ImageLab.java if the method is moved to another class, so the source code may need to be changed there too. I'd say that moving the play method to another class and then changing how it is called so that it uses the new class would be the most likely implementation, given that it is the most direct way to refactor the current code. Having the current ImgProvider.play() method simply dispatch to a new method in a new class would work as well, but seems less elegant and also doesn't solve the initial problem of moving audio related functions away from a class which ostensibly is for providing image display functions.

There is no "play button" in the current application.

As above, this was a reference to the menu item under "File>Play"

What does "Audio can use current image to generate music" mean? What is the alternative?

This is also a reference to the current behavior of the ImgProvider.play() method. Currently, ImgProvider.play() uses other ImgProvider methods in order to get Image information and then creates a small song based on the RGB values in the image. In order to maintain current functionality, if the play() method is moved to another class, it needs to be able to access that information from ImgProvider. This shouldn't be too much of an issue because the methods used, getRed(), getGreen(), and getBlue() are all public, but any implementation of the play() method should be able to either be fed an instance of ImgProvider via an argument, or have access to an instance of ImgProvider via fields in its own class.

[jody]

All replies make sense. Please either create a new issue or revise this one. Thanks!

[nicholasmatthews-dev]

User story updated to fix problems with previous acceptance criteria.

[jody]

Modified title to clarify that this is a refactoring effort.