Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Story/geco 122 #23

Open
wants to merge 36 commits into
base: develop
Choose a base branch
from
Open

Story/geco 122 #23

wants to merge 36 commits into from

Conversation

abhis1989kumar
Copy link

No description provided.

Abhishek Kumar and others added 27 commits September 13, 2018 15:44
@Autowired
private IKafkaRequestSender kafkaRequestSender;

@Autowired
private ISystemMessageHandler messageHandler;

@Autowired
private SystemMessageHandler msgHandler;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is already autowired

page.setOCRType(Properties.OCR_HOCR);
} else {
page.setOCRType(Properties.OCR_PLAINTEXT);
page.setLanguageType(defaultLang);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do you not set the default language if ocr type is HOCR?

try {
Process proc;
BufferedReader reader;
proc = Runtime.getRuntime().exec(command);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merge with 359

output = output + line + " ";
}
proc.waitFor();
reader.close();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the reader always needs to be closed, even if an exception is thrown.

reader = new BufferedReader(new InputStreamReader(proc.getInputStream()));
String line = "";
while ((line = reader.readLine()) != null) {
output = output + line + " ";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use stringbuffer here

lang_list = output.split(":");

String[] languages;
languages = lang_list[1].split(" ");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why don't you read the the output into a list right away since it's one language per line, isn't it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no string manipulations necessary

@jdamerow jdamerow closed this Oct 30, 2018
@abhis1989kumar abhis1989kumar reopened this Nov 1, 2018
@jdamerow
Copy link
Member

jdamerow commented Nov 1, 2018

there don't seem to be any changes

@jdamerow jdamerow closed this Nov 1, 2018
Abhishek Kumar added 2 commits November 1, 2018 15:20
@abhis1989kumar abhis1989kumar reopened this Nov 1, 2018
for (int i = 1; i < langs.size(); i++) {
langTypeMap.put(langs.get(i), langs.get(i));
}
tessPars = null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do you do this?

page.setOCRType(Properties.OCR_HOCR);
} else {
page.setOCRType(Properties.OCR_PLAINTEXT);
}

page.setLanguageType(defaultLang);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's move that to line 76 to keep it together with the rest of the default language code

* @param propertyManager
* @return String[] - the list of available languages
*/
public List<String> getTessLangs(IPropertiesManager propertyManager) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's not introduce a new dependency here, just use the TesseractOCRConfig class used in the other methods.

sysMsgHandler.handleMessage("Error while getting Tesserract languages.", e, MessageType.ERROR);
} catch (InterruptedException e) {
sysMsgHandler.handleMessage("Error while getting Tesserract languages.", e, MessageType.ERROR);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this needs a finally block in which you close the reader

String line = "";
while ((line = reader.readLine()) != null) {
output.add(line);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you still need a comment here, explaining why you are doing what you're doing. and wasn't the first line, not a language?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will add the comments.
The first line is "List of available languages:(3)" and therefore it is not required.

@jdamerow jdamerow closed this Nov 1, 2018
Abhishek Kumar added 2 commits November 2, 2018 15:48
@abhis1989kumar abhis1989kumar reopened this Nov 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants