Говнокод или нет?

Question

[SlideUp]

Ребятки всем привет. Решаю разные задачки с помощью JS и вот решил одну(квадратное уравнение). Подкажите нормальный ли код или лучше как-то по ф-циям разбить?

let a = prompt("Enter a value", 10);
let b = prompt("Enter b value", 10);
let c = prompt("Enter c value", 10);
let result;
let discr = (b * b ) - 4 * a * c;
let sqrtDiscr = Math.sqrt(discr);
if(a === 0 && b === 0 && c === 0){
  throw new Error("The values must be bigger than 0");
} else if(discr < 0 ){
  throw new Error("This Equation have not solution");
} else if(discr === 0 ){
  console.log("This Equation have only 1 solution")
  result = (-b) / (2 * a);
} else if (discr > 0){
  console.log("This Equation have 2 solution");
  result = (- b + sqrtDiscr) / (2 * a) + ";     " + (- b - sqrtDiscr) / (2 * a);
}

console.log(result);

Comments

[Andrey Perov]

SlideUp по мне так удобнее и код не очень

let a =1,
     b = 2,
     c = 3;

Выносите в функцию это, например то что считает и возвращайте return результат ее

[SlideUp]

Andrey Perov, Лет подправил, а код не очень через if'ы?

[Алексей Шашенков]

Я так понял до функций еще не дошли?

[Andrey Perov]

SlideUp лучше делать код логичным более и разделять на куски, на функции каждая функция должна заниматься какой то своей задачей

[ⓒⓢⓢ]

Так и планировалось при значениях a=0,b=1,c=1 выводить

This Equation have 2 solution
NaN; -Infinity

Или это только на моём браузере так...

[SlideUp]

JRK_DV, Принял ошибку, спасибо!

Answer 1

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

Код г-но неидеален, и вот, почему:

см. комменты в коде

let a = prompt("Enter a value", 10); // a, b, c дальше не меняются
let b = prompt("Enter b value", 10); // поэтому можно
let c = prompt("Enter c value", 10);  // const
let result;
let discr = (b * b ) - 4 * a * c;
let sqrtDiscr = Math.sqrt(discr); // при отрицат. discr здесь NaN
if(a === 0 && b === 0 && c === 0) { // проверяем уже после вычислений
  throw new Error("The values must be bigger than 0");
} else if(discr < 0 ){ // проверяем уже после вычисления sqrtDiscr
  throw new Error("This Equation have not solution");
} else if(discr === 0 ){
  console.log("This Equation have only 1 solution")
  result = (-b) / (2 * a);
} else if (discr > 0){
  console.log("This Equation have 2 solution");
  result = (- b + sqrtDiscr) / (2 * a) + ";     " + (- b - sqrtDiscr) / (2 * a);
}

console.log(result);

По функциям разбивать есть смысл если из нескольких мест идёт обращение, код повторяется более двух раз. Тут можно в двух последних случаях считать два корня одной формулой, и при нулевом дискриминанте брать только одно из двух равных значений. Но это излишняя, дурная, вредная переоптимизация.

Ещё вы не проверили случай, когда a === 0: у вас будет деление на ноль.

Comments

[SlideUp]

Спасибо!

[SlideUp]

У меня еще и ошибка была, я же с промта получал строку и не переводил ёё в число.
Спасибо вам за детальный ответ)

Answer 2

[Илья Герасимов]

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

Comments

[SlideUp]

Окей, принял. Спасибо!)

Answer 3

[Павел Корнилов]

Я бы сделал как-то так:

<button onclick="go();">Пуск</button>
<script>
	function go () {
		let t= " in ax2 + bx + c",
			a = prompt("Enter a value"+t, 10),
			b = prompt("Enter b value"+t, 10),
			c = prompt("Enter c value"+t, 10),
			result;
		
		if(a === 0 && b === 0 && c === 0){
			return alert ("The values must be bigger than 0");
		}

		var discr = (b * b ) - 4 * a * c,
			sqrtDiscr = Math.sqrt(discr);
		
		if(discr < 0 ){
			return alert ("This Equation have not solution");
		}

		

		if(discr === 0 ){
		console.log("This Equation have only 1 solution")
		result = (-b) / (2 * a);
		} else {
		console.log("This Equation have 2 solution");
		result = ((- b + sqrtDiscr) / (2 * a)).toFixed(2) + "; \n" + ((- b - sqrtDiscr) / (2 * a)).toFixed(2);
		alert (result);
		}
	}

Comments

[SlideUp]

На самом деле, проверку if(a === 0 && b === 0 && c === 0) можна заменить просто на a === 0, потому как, если a будет 0, то будет потом происходить деление на 0 (что даст инфинити).

В общем я передал вот так =>

const a = Number(prompt("Enter a value")),
b = Number(prompt("Enter b value")),
c = Number(prompt("Enter c value"));
let sqrtDiscr, result;
if(a === 0) throw new Error("The value must be bigger than 0");
let discr = (b * b ) - 4 * a * c;
if(discr < 0 ){
  throw new Error("discr must be positive number");
} else if (discr === 0){
  console.log("This Equation have only 1 solution")
  result = (-b) / (2 * a);
} else if (discr > 0){
  sqrtDiscr = Math.sqrt(discr);
  console.log("This Equation have 2 solution");
  result = (- b + sqrtDiscr) / (2 * a) + ";     " + (- b - sqrtDiscr) / (2 * a);
}

console.log(result);

[Павел Корнилов]

Математически не совсем согласен. При а=0 останется линейное уравнение прямой bx+c, которое даст 1 решение.

[SlideUp]

KorniloFF, Хмм..если ввести a 0 то результат при любых значениях b и с будет NaN и Infinity

[Павел Корнилов]

Да, я знаю. Значит и формулу стоит доработать, или условие еще одно прописать.
А так да - result = (-b) / (2 * a) - не потерпит а=0.