Hej, ma ktoś może pomysł jak inaczej zapisać poniższy kod? Bo wydaje mi się on mało profesjonalny :). Chyba, ze uważacie, że ten kod jest jak najbardziej prawidłowy to go zostawię
float feval(string f, float n)
{
if(f == "sin")
{
return sin(n);
}
if(f == "cos")
{
return cos(n);
}
return 0.0;
}
@dcielak: Argument typu 'string' możesz zmienić na 'const string&'.
Zmiast 2 x if użyj else if, albo switch'a jak zaproponowano wyżej.
a zamiast switcha można to inaczej zapisac?
@dcielak: Napisałem wyżej, jeżeli nie chcesz switcha to zrób:
if()
{
}
else if()
{
}
Hej, ma ktoś może pomysł jak inaczej zapisać poniższy kod?
W ogóle go nie pisać - tylko użyć bezpośrednio **sin **, **cos **zamiast feval
@dcielak: Na upartego możesz użyć jeszcze użyć 'unordered_map' mapującej string na wskaźniki do poszczególnych funkcji.
myslalem, ze moze mozna uzyc jakos operatoru trojargumentowego, ale raczej sensu nie ma
Czy ktos moze zerknąć czy switch zostal poprawnie uzyty?
float feval(string f, float n)
{
switch(f){
case "sin": { return sin(n); } break;
case "cos" : { return cos(n); } break;
default : { return 0.0; } break;
}
}
@dcielak: Nie pamiętam teraz dokładnie składni wskaźników na funkcje, więc sobie sprawdź, ale wyglądałoby to mniej więcej tak:
std::unordered_map<std::string, func_ptr> trig_func;
I dodajesz sobie funkcje:
trig_func["sin"] = ...wskaźnik na funkcję sinus etc.
@Eldorad O.: prawie ;)
@dcielak nie wiem którą wersję standardu C++ masz dostępną, od 17 można zrobić tak:
#include <unordered_map>
#include <cmath>
#include <string_view>
using namespace std::literals;
float eval_func(const std::string_view s, float num)
{
static const std::unordered_map<std::string_view, float (*) (float)> funcs {
{"sin"sv, std::sin},
{"cos"sv, std::cos}
};
auto func = funcs.find(s);
return func == end(funcs) ? 0.0f : func->second(num);
}
int main(int, char*[])
{
eval_func("sin", 5);
}
EDIT: tylko kwestia jeszcze co to znaczy "prawidłowy"? Ja wolę mapy zamiast ifologii/switch-case ale to często kwestia konwencji i dodatkowych wymagań, np. wydajności. Także tu nie da się odpowiedzieć wprost "tak jest lepiej/gorzej".
Na pewno dobrze używać referencji do stringa żeby go bez sensu nie kopiować (string_view jest taką właśnie referencją schowaną za obiektem).
Dobrą praktyką jest też robienie constem (albo in constexpr) czego się da.
Tylko żeby była jasność: jak masz proste if-else lda jednego warunku to robienie mapy może być armatą na muchę. Jak masz przypadków więcej to szala się imho przechyla w kierunku mapy. Ale jak zwykle - kontekst.
Ja zacząłbym od pytania bracie @dcielak do czego potrzebujesz istnienia funkcji fval().
wykorzystuję ją w tym miejscu:
for(int k=1; k<=n+1; ++k)
{
X[k] = cos((2.0*k - 1)*d);
X[k] = (b-a)*(X[k]/2.0)+((a+b)/2.0);
Y[k] = feval(f,X[k]);
}
Hmmm masz tutaj w Peruna zmiennych wyrwanych z kontekstu - a, b, n, f...może inaczej, weź opisowo wyjaśnij jaki program piszesz i jaki efekt chcesz osiągnąć, bo to mi podejrzanie wygląda na problem XY.
Chodziło mi tylko o inny zapis tych warunków, probowalem switcha uzyc, ale on nie dziala dla stringow
Jeśli Twój przykład to faktyczny przypadek użycia, to zamiast przesyłania funkcji do wykonania w postaci stringu, przekaż po prostu wskaźnik na funkcję. Na pewno będzie bardziej elegancko i bez sprawdzania co iterację po stringach.
Żeby to zrobić jeszcze inaczej, może lepiej, trzeba znać tak naprawdę szerszy kontekst: jak i dlaczego "f" jest ustawiane na string "sin" / "cos".
dcielak napisał(a):
Chodziło mi tylko o inny zapis tych warunków, probowalem switcha uzyc, ale on nie dziala dla stringow
Zmień sterowanie nie poprzez stringa, ale poprzez jakiegoś enuma:
enum Operation
{
Sinus = 0,
Cosinus
};
teraz funkcja wybierająca operację winna być zapisana tak:
float eval_func(Operation operationToPreform, float number)
{
switch (operationToPreform)
case Sinus:
break;
default:;
}
Nie zmienia to jednak faktu, iż nadal nie wiadomo po Swaroga kombinować z funkcją skoro można zapisać to tak:
Y[k] = f == "sin" ? sin(X[k]) : cos(X[k]);