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.

Sign up for GitHub

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

Korean future fix #40

Merged
merged 4 commits into from
Apr 25, 2017
Merged

Korean future fix #40

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3139277
corrections to future tense
cecilebertin Apr 25, 2017
6c94bc8
added future tense tests
cecilebertin Apr 25, 2017
07ad3c7
koreanFuture space fix
cecilebertin Apr 25, 2017
6c1eb3d
koreanFuture ending fix
cecilebertin Apr 25, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 15 additions & 7 deletions src/korean.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ const breakdown = (input) => {
// input: array of 2 or 3 numbers representing modern jamo (components) that make up a hangul syllable
// output: a hangul character
const combineSymbols = (input) => {
const initialValue = input[0] * 588;
const medialValue = input[1] * 28;
const finalValue = input[2] ? input[2] : 0;
const total = initialValue + medialValue + finalValue + 44032;
const finalWord = String.fromCharCode(total);
let unicodeTotal = (input[0] * 588) + (input[1] * 28) + 44032;
if(input.length === 3) {
unicodeTotal += input[2];
}
const finalWord = String.fromCharCode(unicodeTotal);
Copy link
Contributor

Choose a reason for hiding this comment

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

can we simply return String.fromCharCode(unicodeTotal) without assigning?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@minsooshin you're right. I'll fix that right away.

return finalWord;
};

Expand Down Expand Up @@ -129,10 +129,18 @@ class Korean {

doFuture (word) {
const stem = breakdown(word.slice(0, -1));
if (stem[-1] !== 8) {
if (stem.length < 3) {
stem.push(8);
return `${combineSymbols(stem)}꺼야`;
} else {
if (stem[stem.length-1] === 17) {
stem.pop();
return `${combineSymbols(stem)}울꺼야`;
} else if (stem[stem.length-1] === 8) {
return `${combineSymbols(stem)}꺼야`;
}
return `${combineSymbols(stem)}을꺼야`;
}
return `${combineSymbols(stem)}꺼야`;
}
} // end for class

Expand Down
48 changes: 34 additions & 14 deletions test/korean_tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,28 +107,48 @@ describe('Korean', () => {
});
});
describe('Future Tense', () => {
it('should conjugate regular verbs correctly', () => {
it('should conjugate verbs with stem ending with a vowel correctly', () => {
const kc = new Korean();
let presentWord = kc.conjugate('하다', {tense: 'future'});
expect(presentWord).to.equal('할꺼야');
let futureWord = kc.conjugate('하다', {tense: 'future'});
expect(futureWord).to.equal('할꺼야');
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should be 할 거야. 할꺼야 is pronunciation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@minsooshin Thanks! I'll add a fix for this too.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@minsooshin I looked it up, http://www.koreanwikiproject.com/wiki/A/V_%2B_(으)ㄹ_거예요
I think there should be a space in between for all the cases.

Copy link
Contributor

@minsooshin minsooshin Apr 25, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@cecilebertin that's right. Also, the example you provide is to senior people (Informal polite). So, for your test cases, I think To make the low form (반말), the 예요 part becomes 야: A/V+(으)ㄹ 거야 is the correct reference. That's what I said before; it should be 할 거야. There is a whitespace between 할 and 거야.


presentWord = kc.conjugate('오다', {tense: 'future'});
expect(presentWord).to.equal('올꺼야');
futureWord = kc.conjugate('오다', {tense: 'future'});
expect(futureWord).to.equal('올꺼야');

presentWord = kc.conjugate('비다', {tense: 'future'});
expect(presentWord).to.equal('빌꺼야');
futureWord = kc.conjugate('비다', {tense: 'future'});
expect(futureWord).to.equal('빌꺼야');
});
it('should conjugate verbs with stem ending with a ㄹ consonant correctly',
() => {
const kc = new Korean();
let futureWord = kc.conjugate('놀다', {tense: 'future'});
expect(futureWord).to.equal('놀꺼야');

futureWord = kc.conjugate('날다', {tense: 'future'});
expect(futureWord).to.equal('날꺼야');

futureWord = kc.conjugate('울다', {tense: 'future'});
expect(futureWord).to.equal('울꺼야');
});
it('should conjugate words with ㄹ final vowel correctly', () => {
it('should conjugate verbs with stem ending with ㅂ consonant correctly',
() => {
const kc = new Korean();
let presentWord = kc.conjugate('놀다', {tense: 'future'});
expect(presentWord).to.equal('놀꺼야');
let futureWord = kc.conjugate('춥다', {tense: 'future'});
expect(futureWord).to.equal('추울꺼야');

presentWord = kc.conjugate('날다', {tense: 'future'});
expect(presentWord).to.equal('날꺼야');
futureWord = kc.conjugate('덥다', {tense: 'future'});
expect(futureWord).to.equal('더울꺼야');
});
it('should conjugate verbs with stem ending with other consonants correctly',
() => {
const kc = new Korean();
let futureWord = kc.conjugate('있다', {tense: 'future'});
expect(futureWord).to.equal('있을꺼야');

presentWord = kc.conjugate('울다', {tense: 'future'});
expect(presentWord).to.equal('울꺼야');
futureWord = kc.conjugate('먹다', {tense: 'future'});
expect(futureWord).to.equal('먹을꺼야');
});

});
describe('Present Continuous Tense', () => {
it('should conjugate verbs correctly', () => {
Expand Down