Как можно улучшить представленный код?

Question

[maxemga]

function func(s, a, b) {

    if (s.match(/^$/)) {
        return -1;
    }

    var i = s.length -1;
    var aIndex = -1;
    var bIndex = -1;

    while ((aIndex == -1) && (bIndex == -1) && (i > 0)) {

        if (s.substring(i, i +1) == a) {
            aIndex = i;
        }
        if (s.substring(i, i +1) == b) {
            bIndex = i;
        }
        i = i - 1;

    }

    if (aIndex != -1) {
        if (bIndex == -1) {
            return aIndex;
        }
        else {
            return Math.max(aIndex, bIndex);
        }
    }

    if (bIndex != -1) {
        return bIndex;
    }
    else {
        return -1;
    }
}

Comments

[Сергей Соколов]

это мы за вас собеседование или учебно-тестовое задание проходим?

Что вам кажется здесь требующим улучшения?

[maxemga]

Сергей Соколов, проходил собеседование и там было представленно данное задание, я его решил и сейчас хочу посмотреть на то как другие люди справились бы с данным заданием и где я мог еще лучше оптимизировать данный код.

[Сергей Соколов]

maxemga, задание как сформулировано?

[Сергей Сергей]

Встроенные методы нельзя использовать?

Я недавно посмотрел правила

Это опрос и это НАРУШЕНИЕ.

[WbICHA]

maxemga, имхо, с таким уровнем написания кода тебе ещё очень рано по собеседованиям ходить.
Тут и использование варов, и, главное, проблемы с написанием логики.

if (s.match(/^$/)) {

Вот это, к примеру, худшая проверка на пустую строку из всех, что видел.

if (bIndex != -1) {
        return bIndex;
    }
    else {
        return -1;
    }

Вот эта конструкция вообще не имеет никакого смысла, она вообще ничего не делает.
Да и в целом вот эта вся конструкция:

if (aIndex != -1) {
        if (bIndex == -1) {
            return aIndex;
        }
        else {
            return Math.max(aIndex, bIndex);
        }
    }

    if (bIndex != -1) {
        return bIndex;
    }
    else {
        return -1;
    }

Заменяется на:

return Math.max(aIndex, bIndex);
// or
return aIndex > bIndex ? aIndex : bIndex;

while ((aIndex == -1) && (bIndex == -1) && (i > 0)) {

        if (s.substring(i, i +1) == a) {
            aIndex = i;
        }
        if (s.substring(i, i +1) == b) {
            bIndex = i;
        }
        i = i - 1;

    }

А это просто лютый говнокод.

Серьёзно, тебе ещё учиться и учиться.

[maxemga]

WbICHA, друг, код который я представил не является моим кодом, я взял код с задания и его суть как раз таки его улучшить и привести в нормальный вид.

[WbICHA]

maxemga, в таком случае тебе стоило бы научиться выражать свои мысли.

[maxemga]

WbICHA, хорошо, я поработаю над этим.

Answer 1

[Сергей Соколов]

const func = (s, a, b) => Math.max(s.lastIndexOf(a), s.lastIndexOf(b));

Comments

[maxemga]

Хм.... Это поистине гениально

Answer 2

[TheSnegok]

const func = (s, a, b) => {
    if (s.match(/^$/)) {
        return -1;
    }

    let aIndex = -1;
    let bIndex = -1;

if(aIndex === -1 && bIndex  === -1) {
   for(let i = s.length -1;i > 0;i--) {
        if (s.substring(i, i +1) === a) {
            aIndex = i;
        }
        if (s.substring(i, i +1) === b) {
            bIndex = i;
        }
    }
}

    if (aIndex !== -1) {
        return bIndex === -1 ? aIndex :  Math.max(aIndex, bIndex);
    }
    return bIndex !== -1 ? bIndex : -1;

На работоспособность не проверял!

Comments

[Сергей Соколов]

bIndex != -1 ? bIndex : return -1;

это что такое ?!

[Сергей Соколов]

bIndex !== -1 ? bIndex : -1;

return где-то потеряли.

[TheSnegok]

Сергей Соколов, Попутал с стрелочной функцией, благодарю)

[Сергей Соколов]

bIndex !== -1 ? return bIndex : return -1;

@TheSnegok, сделали только хуже.

Пробегитесь по этой главе.
условие ? значение1 : значение2 — это выражение. Его (целиком) надо вернуть, тогда уж.