# TranslateIt class using the Yandex translation service

I wrote a class TranslateIt for translating text files with Yandex.Translate service. I feel the code is rather bulky. Is it better to decompose it to several methods? I'm eager to know opinions of experienced people.

import requests
import os
import chardet

class TranslateIt(object):
"""
methods
translateit (source, **kwargs)
translateit - translates source into output
source - txt file to be translated
froml - source language, if not specified, yandex tries to autodetect
tol - translation language, default is ru
output - translayed txt file

attributes
params - parameters dict for yandex api
encoding - encoding of source file detected by _checkEncoding method
source_name, output_name - absolute paths of source and output files respectively
code - result of translation
message - error message if translation failed
translated = translated text if succeeded

constants
MAX_SIZE - maximum size of a source file according to yandex api
"""

key = 'trnsl.1.1.20170331T021116Z.53a353fef9d94e08.9617ecfdaf7fbe170972c4107aac8780e53dd21f'
api_url = 'https://translate.yandex.net/api/v1.5/tr.json/translate'
MAX_SIZE = 10000
yandex_string = "\nTranslated by Yandex.Translate service, 'http://translate.yandex.ru/'" #added to translation result according to licence requirements

#check the encoding of a source file
def _checkEncoding(self, file_name):
open(file_name).close()
return result['encoding']

#translates source to output
def translateIt(self, source, **kwargs):
self.params = {'key': TranslateIt.key}
#check if kwargs are valid
kwargs_tuple = ('output', 'froml', 'tol')
if not all([key in kwargs_tuple for key in kwargs.keys()]):
raise NameError ('Invalid named parameter')
# check if source file exists
if os.path.exists(source) and source.endswith('.txt'):
self.encoding = TranslateIt._checkEncoding(self, source)
self.source_name = os.path.abspath (source)
# if source exists, assign it's content to text parameter
with open(self.source_name, 'r', encoding=self.encoding) as f:
if len (text) < TranslateIt.MAX_SIZE:
self.params['text'] = text
else:
raise ValueError('File size can not be more than {} symbols'.format(TranslateIt.MAX_SIZE))
else:
raise FileNotFoundError('No data source file found')

#create translation pair
if 'tol' not in kwargs.keys():
kwargs['tol'] = 'ru'
if 'from' not in kwargs.keys():
self.params['options'] = 1
self.params['lang'] = kwargs['tol']
else:
self.params['lang'] = kwargs['froml'] + '-' + kwargs['tol']

#create output file:
if 'output' in kwargs.keys():
self.output_name = os.path.abspath(kwargs['output']) if kwargs['output'].endswith('.txt') else os.path.abspath(kwargs['output'] +'.txt')
else:
self.output_name = os.path.abspath('translated_' + self.params['lang'] + '_' + os.path.split(self.source_name)[-1])

#translate, save result in output file if succeeded
response_result = requests.get(TranslateIt.api_url, params=self.params).json()
self.code = response_result['code']
if self.code != 200:
self.message = response_result['message']
print ('Translation failed\n Error code: {0}, error message: {1}'.format(self.code, self.message))
else:
self.translated = response_result['text'][0] + TranslateIt.yandex_string
with open(self.output_name, 'w', encoding='utf8') as f:
f.write(self.translated)
print ('Translation successful, result saved in {0}'.format(self.output_name))

• See Stop Writing Classes. There's no state, you don't even have __init__. – jonrsharpe Apr 2 '17 at 8:51
• Thanks for your feedback! I read an article with the same title. Actually, I am a beginner with a zero base and now trying to learn OOP basics. I supposed using class can give me more flexibility. – Mikhail Belousov Apr 2 '17 at 8:57
• More flexibility than what, and the flexibility to do what? See also the YAGNI principle. – jonrsharpe Apr 2 '17 at 8:58
• Usually flexilibility to re-use and expand code. For me it's more comfortable to use class attributes than variables in a functions. – Mikhail Belousov Apr 2 '17 at 9:07
• Actually I understand what you mean. I saw class examples like CSVWriter class that are only used as instruments for other classes. My questions was not just about rationality of using the class, I'd like also to get the code reviewed as well if somebody has some spare time) – Mikhail Belousov Apr 2 '17 at 9:10

Some ideas:

• If the arguments are a fixed list, don't use **kwargs. I find **kwargs is only really useful if you have some middleware code which needs to forward an arbitrary set of arguments to another piece of code. That way people don't need to rely on your documentation (or when the documentation inevitably gets out of date, the code) to understand which parameters the method takes. Using named parameters you can also get rid of loops over **kwargs.
• IMO attributes and constants shouldn't be documented at all. Any IDE will be able to auto-complete them, and their names should be sufficiently self-explanatory that you don't need extra documentation.
• Starting a method name with check implies an assertion is happening. Use get when that is what you're doing.
• This is a personal style item, but I prefer that the interesting/important code is at the top of the file. _checkEncoding is not that interesting.
• If key is a secret it should not be hard-coded.
• Don't second guess TranslateIt's API. For example, don't verify the max size on the client side. Instead you should rely on the API to tell you when the size is too large and forward the information to the user. This way your implementation is simpler and more importantly doesn't get out of sync with the API.
• You should use the with open() pattern in _checkEncoding.
• Use pep8 to fix style issues like method names.
• Your script is not usable from a command line.
• Why make yandex_string a field? It is only used once, and is never changed, so you might as well treat it as a constant.
• The code is not testable as it stands. This is too big a subject to expand much on here, but suffice it to say that translateIt does much more than one thing.
• Thanks for a feedback! 1. I'm trying to learn using a command line, can't do it for now – Mikhail Belousov Apr 2 '17 at 9:43
• So, it's better to decompose the code into several smaller pieces, right? – Mikhail Belousov Apr 2 '17 at 9:49
• Absolutely. Have a look at Working Effectively with Legacy Code for many tips on how to do it. – l0b0 Apr 2 '17 at 10:01
• Got it, trying to decompose. One more thing: am I wright that I'd better to use argparse for working with a command line? Can you give me a hint where I can read about it? – Mikhail Belousov Apr 2 '17 at 10:03
• The documentation, tutorial and a simple example should give you plenty to work with. – l0b0 Apr 2 '17 at 10:08

I tried to decompose and to use more appropriate names according to pep8. Using command line TBD. Below is just a coda that has been changed.

def _get_encoding(self, file_name):
open(file_name).close()
return result['encoding']

def _get_source(self, source):
if os.path.exists(source) and source.endswith('.txt'):
#check the encoding of a source file
self.encoding = TranslateIt._get_encoding(self, source)
self.source_name = os.path.abspath (source)
# if source exists, assign it's content to text parameter
with open(self.source_name, 'r', encoding=self.encoding) as f:
if len(text) < TranslateIt.MAX_SIZE:
return text
else:
raise ValueError('File size can not be more than {} symbols'.format(TranslateIt.MAX_SIZE))
else:
raise FileNotFoundError('No data source file found')

def _set_langs(froml, tol):
if froml=='auto':
else:
return(froml + '-' + tol), None

def _set_output(self, output):
if output=='same':
self.output_name = os.path.abspath('translated_' + self.params['lang'] + '_' + os.path.split(self.source_name)[-1])
else:
self.output_name = os.path.abspath(output) if output.endswith('.txt') else os.path.abspath(output +'.txt')

def _save_output(output_name, text):
with open(output_name, 'w', encoding='utf8') as f:
f.write(text)

def translate_it(self, source, output='same', froml='auto', tol='ru'):
self.params = {'key': TranslateIt.key}
# get source  file
self.params['text'] = TranslateIt._get_source(self, source)
#create translation pair
self.params['lang'], self.params['options'] = TranslateIt._set_langs(froml, tol)
#create output file
TranslateIt._set_output(self, output)
#translate, save result in output file if succeeded
try:
response_result = requests.get(TranslateIt.api_url, params=self.params).json()
self.code = response_result['code']
if self.code != 200:
self.message = response_result['message']
print('Translation failed\n Error code: {0}, error message: {1}'.format(self.code, self.message))
else:
self.translated_text = response_result['text'][0] + TranslateIt.yandex_string
TranslateIt._save_output(self.output_name, self.translated_text)
print('Translation successful, result saved in {0}'.format(self.output_name))
except Exception:
print("Got unexpected error")
pass