Skip to main content
added 1322 characters in body
Source Link
burna
  • 306
  • 1
  • 7

Overall

Design first.

Name and semantic of your class

I have this code that converts audio to different file formats.

Not exactly, your code is a client to an Encoder, that converts audio to different file formats. In design, it is very important to be certain about the reason and semantic of an object. So first, the name AudioEncoderDecoder is bad. It says nothing about the sematic of the object. I would call it AudioFormatConverter as it converts one audio file format to another.

Public methods

public void encodeAudio(File source, File target, String mimeType);

a) Naming encodeAudio() is not clear, convert() would be clear and consistent with the classname.

b) Parameters: source, target file seems good. But why would I pass in the mimeType? And how can I specify which target format I choose?

Expected method signature:

public void convert(File source, File target) throws AudioConversionException();

One conversion method in the public interface, source and target file, the mime type is guessed by the file extension. You could add another public method, where you take in the mimetype, as needed. But keep the interface of your class as simple, logic, consistent and understandable as possible.

I would declare an checked exception for error handling, formats not avail or whatever.

Design Patterns

If there will be more formats, codecs, mimetypes you could take a look at Chain of responsibility or Dispatcher/Command pattern. These patterns also add flexibility to easily support more mimetypes at runtime. Without those patterns, you may get a mid-to-big if/then/else block.

Possible Design with Command, Strategy pattern applied: Possible design of AudioFormatConverter

  • AudioFormatConverters interface is format-agnostic, this means it is possible to implement a converter, that doesn't use Files but only InputStreams or byte[] arrays.
  • Concrete AudioFileFormatConverter converts from and to Files, and holds a list of supported ConversionCommands
  • ConversionCommand encapsulates logic to convert things, a client can check via isSupported(source, target) if the source-format and target-format is supported by this command (handler would by a common name, too)
  • Mp3ToOggConversionCommand Every conversion is a command, one can decide that this is too much, but with that you can save state in the command, like executation duration, error states, sizes, redo, undo, execute asynchronous...
  • ConversionStrategy with Impl EncoderConversionStrategy encapsulates the concrete implementation of conversion, that is your duplicate code, this strategy how to convert 2 files by using the Encoder

Note that ConversionCommands should be prototypes, so that each execute happens in its own Command instance.

Now in code, you would setup the AudioFileFormatConverter and add the supported ConversionCommands to it.

Existing code

Obey the basic OO principles.

  • DRY - Don't repeat yourself. If you find code is duplicate, introduce abstractions or extract methods.
  • Use interfaces

In your case you can easily improve the code if you generalize the mp3ToOgg and oggToMp3 methods.

So that you have only one general method for converting with the signature:

private void convert(File source, File target, String format, String codec);

Overall

Design first.

Name and semantic of your class

I have this code that converts audio to different file formats.

Not exactly, your code is a client to an Encoder, that converts audio to different file formats. In design, it is very important to be certain about the reason and semantic of an object. So first, the name AudioEncoderDecoder is bad. It says nothing about the sematic of the object. I would call it AudioFormatConverter as it converts one audio file format to another.

Public methods

public void encodeAudio(File source, File target, String mimeType);

a) Naming encodeAudio() is not clear, convert() would be clear and consistent with the classname.

b) Parameters: source, target file seems good. But why would I pass in the mimeType? And how can I specify which target format I choose?

Expected method signature:

public void convert(File source, File target) throws AudioConversionException();

One conversion method in the public interface, source and target file, the mime type is guessed by the file extension. You could add another public method, where you take in the mimetype, as needed. But keep the interface of your class as simple, logic, consistent and understandable as possible.

I would declare an checked exception for error handling, formats not avail or whatever.

Design Patterns

If there will be more formats, codecs, mimetypes you could take a look at Chain of responsibility or Dispatcher/Command pattern. These patterns also add flexibility to easily support more mimetypes at runtime. Without those patterns, you may get a mid-to-big if/then/else block.

Existing code

Obey the basic OO principles.

  • DRY - Don't repeat yourself. If you find code is duplicate, introduce abstractions or extract methods.
  • Use interfaces

In your case you can easily improve the code if you generalize the mp3ToOgg and oggToMp3 methods.

So that you have only one general method for converting with the signature:

private void convert(File source, File target, String format, String codec);

Overall

Design first.

Name and semantic of your class

I have this code that converts audio to different file formats.

Not exactly, your code is a client to an Encoder, that converts audio to different file formats. In design, it is very important to be certain about the reason and semantic of an object. So first, the name AudioEncoderDecoder is bad. It says nothing about the sematic of the object. I would call it AudioFormatConverter as it converts one audio file format to another.

Public methods

public void encodeAudio(File source, File target, String mimeType);

a) Naming encodeAudio() is not clear, convert() would be clear and consistent with the classname.

b) Parameters: source, target file seems good. But why would I pass in the mimeType? And how can I specify which target format I choose?

Expected method signature:

public void convert(File source, File target) throws AudioConversionException();

One conversion method in the public interface, source and target file, the mime type is guessed by the file extension. You could add another public method, where you take in the mimetype, as needed. But keep the interface of your class as simple, logic, consistent and understandable as possible.

I would declare an checked exception for error handling, formats not avail or whatever.

Design Patterns

If there will be more formats, codecs, mimetypes you could take a look at Chain of responsibility or Dispatcher/Command pattern. These patterns also add flexibility to easily support more mimetypes at runtime. Without those patterns, you may get a mid-to-big if/then/else block.

Possible Design with Command, Strategy pattern applied: Possible design of AudioFormatConverter

  • AudioFormatConverters interface is format-agnostic, this means it is possible to implement a converter, that doesn't use Files but only InputStreams or byte[] arrays.
  • Concrete AudioFileFormatConverter converts from and to Files, and holds a list of supported ConversionCommands
  • ConversionCommand encapsulates logic to convert things, a client can check via isSupported(source, target) if the source-format and target-format is supported by this command (handler would by a common name, too)
  • Mp3ToOggConversionCommand Every conversion is a command, one can decide that this is too much, but with that you can save state in the command, like executation duration, error states, sizes, redo, undo, execute asynchronous...
  • ConversionStrategy with Impl EncoderConversionStrategy encapsulates the concrete implementation of conversion, that is your duplicate code, this strategy how to convert 2 files by using the Encoder

Note that ConversionCommands should be prototypes, so that each execute happens in its own Command instance.

Now in code, you would setup the AudioFileFormatConverter and add the supported ConversionCommands to it.

Existing code

Obey the basic OO principles.

  • DRY - Don't repeat yourself. If you find code is duplicate, introduce abstractions or extract methods.
  • Use interfaces

In your case you can easily improve the code if you generalize the mp3ToOgg and oggToMp3 methods.

So that you have only one general method for converting with the signature:

private void convert(File source, File target, String format, String codec);
deleted 106 characters in body
Source Link
burna
  • 306
  • 1
  • 7

Overall

Design first. It doesn't seem that you thought about design of your code first, it looks more like a quick test client.

Name and semantic of your class

I have this code that converts audio to different file formats.

Not exactly, your code is a client to an Encoder, that converts audio to different file formats. In design, it is very important to be certain about the reason and semantic of an object. So first, the name AudioEncoderDecoder is bad. It says nothing about the sematic of the object. I would call it AudioFormatConverter as it converts one audio file format to another.

Public methods

public void encodeAudio(File source, File target, String mimeType);

a) Naming encodeAudio() is not clear, convert() would be clear and consistent with the classname.

b) Parameters: source, target file seems good. But why would I pass in the mimeType? And how can I specify which target format I choose?

Expected method signature:

public void convert(File source, File target) throws AudioConversionException();

One conversion method in the public interface, source and target file, the mime type is guessed by the file extension. You could add another public method, where you take in the mimetype, as needed. But keep the interface of your class as simple, logic, consistent and understandable as possible.

I would declare an checked exception for error handling, formats not avail or whatever.

Design Patterns

If there will be more formats, codecs, mimetypes you could take a look at Chain of responsibility or Dispatcher/Command pattern. These patterns also add flexibility to easily support more mimetypes at runtime. Without those patterns, you may get a mid-to-big if/then/else block.

Existing code

Obey the basic OO principles.

  • DRY - Don't repeat yourself. If you find code is duplicate, introduce abstractions or extract methods.
  • Use interfaces

In your case you can easily improve the code if you generalize the mp3ToOgg and oggToMp3 methods.

So that you have only one general method for converting with the signature:

private void convert(File source, File target, String format, String codec);

Overall

Design first. It doesn't seem that you thought about design of your code first, it looks more like a quick test client.

Name and semantic of your class

I have this code that converts audio to different file formats.

Not exactly, your code is a client to an Encoder, that converts audio to different file formats. In design, it is very important to be certain about the reason and semantic of an object. So first, the name AudioEncoderDecoder is bad. It says nothing about the sematic of the object. I would call it AudioFormatConverter as it converts one audio file format to another.

Public methods

public void encodeAudio(File source, File target, String mimeType);

a) Naming encodeAudio() is not clear, convert() would be clear and consistent with the classname.

b) Parameters: source, target file seems good. But why would I pass in the mimeType? And how can I specify which target format I choose?

Expected method signature:

public void convert(File source, File target) throws AudioConversionException();

One conversion method in the public interface, source and target file, the mime type is guessed by the file extension. You could add another public method, where you take in the mimetype, as needed. But keep the interface of your class as simple, logic, consistent and understandable as possible.

I would declare an checked exception for error handling, formats not avail or whatever.

Design Patterns

If there will be more formats, codecs, mimetypes you could take a look at Chain of responsibility or Dispatcher/Command pattern. These patterns also add flexibility to easily support more mimetypes at runtime. Without those patterns, you may get a mid-to-big if/then/else block.

Existing code

Obey the basic OO principles.

  • DRY - Don't repeat yourself. If you find code is duplicate, introduce abstractions or extract methods.
  • Use interfaces

In your case you can easily improve the code if you generalize the mp3ToOgg and oggToMp3 methods.

So that you have only one general method for converting with the signature:

private void convert(File source, File target, String format, String codec);

Overall

Design first.

Name and semantic of your class

I have this code that converts audio to different file formats.

Not exactly, your code is a client to an Encoder, that converts audio to different file formats. In design, it is very important to be certain about the reason and semantic of an object. So first, the name AudioEncoderDecoder is bad. It says nothing about the sematic of the object. I would call it AudioFormatConverter as it converts one audio file format to another.

Public methods

public void encodeAudio(File source, File target, String mimeType);

a) Naming encodeAudio() is not clear, convert() would be clear and consistent with the classname.

b) Parameters: source, target file seems good. But why would I pass in the mimeType? And how can I specify which target format I choose?

Expected method signature:

public void convert(File source, File target) throws AudioConversionException();

One conversion method in the public interface, source and target file, the mime type is guessed by the file extension. You could add another public method, where you take in the mimetype, as needed. But keep the interface of your class as simple, logic, consistent and understandable as possible.

I would declare an checked exception for error handling, formats not avail or whatever.

Design Patterns

If there will be more formats, codecs, mimetypes you could take a look at Chain of responsibility or Dispatcher/Command pattern. These patterns also add flexibility to easily support more mimetypes at runtime. Without those patterns, you may get a mid-to-big if/then/else block.

Existing code

Obey the basic OO principles.

  • DRY - Don't repeat yourself. If you find code is duplicate, introduce abstractions or extract methods.
  • Use interfaces

In your case you can easily improve the code if you generalize the mp3ToOgg and oggToMp3 methods.

So that you have only one general method for converting with the signature:

private void convert(File source, File target, String format, String codec);
deleted 2 characters in body
Source Link
burna
  • 306
  • 1
  • 7

Overall

Design first. It doesn't seem that you thought about design of your code first, it looks more like a quick test client.

Name and semantic of your class

I have this code that converts audio to different file formats.

This is wrongNot exactly, your code is a client to an Encoder, that converts audio to different file formats. In design, it is very important to be certain about the reason and semantic of an object. So first, the name AudioEncoderDecoder is bad. It says nothing about the sematic of the object. I would call it AudioFormatConverter as it converts one audio file format to another.

Public methods

public void encodeAudio(File source, File target, String mimeType);

a) Naming encodeAudio() is not clear, convert() would be clear and consistent with the classname.

b) Parameters: source, target file seems good. But why would I pass in the mimeType? And how can I specify which target format I choose?

Expected method signature:

public void convert(File source, File target) throws AudioConversionException();

One conversion method in the public interface, source and target file, the mime type is guessed by the file extension. You could add another public method, where you take in the mimetype, as needed. But keep the interface of your class as simple, logic, consistent and understandable as possible.

I would declare an checked exception for error handling, formats not avail or whatever.

Design Patterns

If there will be more formats, codecs, mimetypes you could take a look at Chain of responsibility or Dispatcher/Command pattern. These patterns also add flexibility to easily support more mimetypes at runtime. Without those patterns, you may get a mid-to-big if/then/else block.

Existing code

Obey the basic OO principles.

  • DRY - Don't repeat yourself. If you find code is duplicate, introduce abstractions or extract methods.
  • Use interfaces

In your case you can easily improve the code if you generalize the mp3ToOgg and oggToMp3 methods.

So that you have only one general method for converting with the signature:

private void convert(File source, File target, String format, String codec);

Overall

Design first. It doesn't seem that you thought about design of your code first, it looks more like a quick test client.

Name and semantic of your class

I have this code that converts audio to different file formats.

This is wrong, your code is a client to an Encoder, that converts audio to different file formats. In design, it is very important to be certain about the reason and semantic of an object. So first, the name AudioEncoderDecoder is bad. It says nothing about the sematic of the object. I would call it AudioFormatConverter as it converts one audio file format to another.

Public methods

public void encodeAudio(File source, File target, String mimeType);

a) Naming encodeAudio() is not clear, convert() would be clear and consistent with the classname.

b) Parameters: source, target file seems good. But why would I pass in the mimeType? And how can I specify which target format I choose?

Expected method signature:

public void convert(File source, File target) throws AudioConversionException();

One conversion method in the public interface, source and target file, the mime type is guessed by the file extension. You could add another public method, where you take in the mimetype, as needed. But keep the interface of your class as simple, logic, consistent and understandable as possible.

I would declare an checked exception for error handling, formats not avail or whatever.

Design Patterns

If there will be more formats, codecs, mimetypes you could take a look at Chain of responsibility or Dispatcher/Command pattern. These patterns also add flexibility to easily support more mimetypes at runtime. Without those patterns, you may get a mid-to-big if/then/else block.

Existing code

Obey the basic OO principles.

  • DRY - Don't repeat yourself. If you find code is duplicate, introduce abstractions or extract methods.
  • Use interfaces

In your case you can easily improve the code if you generalize the mp3ToOgg and oggToMp3 methods.

So that you have only one general method for converting with the signature:

private void convert(File source, File target, String format, String codec);

Overall

Design first. It doesn't seem that you thought about design of your code first, it looks more like a quick test client.

Name and semantic of your class

I have this code that converts audio to different file formats.

Not exactly, your code is a client to an Encoder, that converts audio to different file formats. In design, it is very important to be certain about the reason and semantic of an object. So first, the name AudioEncoderDecoder is bad. It says nothing about the sematic of the object. I would call it AudioFormatConverter as it converts one audio file format to another.

Public methods

public void encodeAudio(File source, File target, String mimeType);

a) Naming encodeAudio() is not clear, convert() would be clear and consistent with the classname.

b) Parameters: source, target file seems good. But why would I pass in the mimeType? And how can I specify which target format I choose?

Expected method signature:

public void convert(File source, File target) throws AudioConversionException();

One conversion method in the public interface, source and target file, the mime type is guessed by the file extension. You could add another public method, where you take in the mimetype, as needed. But keep the interface of your class as simple, logic, consistent and understandable as possible.

I would declare an checked exception for error handling, formats not avail or whatever.

Design Patterns

If there will be more formats, codecs, mimetypes you could take a look at Chain of responsibility or Dispatcher/Command pattern. These patterns also add flexibility to easily support more mimetypes at runtime. Without those patterns, you may get a mid-to-big if/then/else block.

Existing code

Obey the basic OO principles.

  • DRY - Don't repeat yourself. If you find code is duplicate, introduce abstractions or extract methods.
  • Use interfaces

In your case you can easily improve the code if you generalize the mp3ToOgg and oggToMp3 methods.

So that you have only one general method for converting with the signature:

private void convert(File source, File target, String format, String codec);
added 31 characters in body
Source Link
burna
  • 306
  • 1
  • 7
Loading
added 142 characters in body
Source Link
burna
  • 306
  • 1
  • 7
Loading
Source Link
burna
  • 306
  • 1
  • 7
Loading