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

fix Compose's domain and some other minor issues in week 11 source #1

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

haki-1
Copy link

@haki-1 haki-1 commented May 12, 2020

No description provided.

float evalAt(float x) override { return f->safeEval(x) + g->safeEval(x); }
};

struct Product : public RealFunc
{
RealFunc *f, *g;
Copy link
Member

Choose a reason for hiding this comment

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

Хубаво е да са поне const, a най-добре и private - да не вземе някой да бърника :)

@@ -160,4 +212,9 @@ int main()

RealFunc* base = ∁
std::cout << base->safeEval(3) << std::endl;
}

// problem with Compose's defined domain - example
Copy link
Member

Choose a reason for hiding this comment

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

Добре де, проблем има, а идея за решение 🤔?


struct TanDomain : Set
{
bool check(float x) const override { return ceilf(x / (M_PI / 2)) != floorf(x / (M_PI / 2)) || (int)(x / (M_PI / 2)) % 2 == 0; }
Copy link
Member

Choose a reason for hiding this comment

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

Това не е ли малко дълго като за един ред 😆.
Освен това е доста криптично написано с тези условия - наложи се да го разпиша формално, за да разбера какво прави 😆
Наистина, опитай по-простичко да го изразиш - трябва просто да провериш дали x е от вида ½π + kπ, т.е. дали уравнението ½π + kπ = x има цяло решение за k.
Как да провериш дали k излиза цяло… ще се сетиш сам. Hint: използвай fmod от cmath.

И като работиш с плаваща запетая, е хубаво да допускаш някаква епсилон грешка. Иначе, формално, това е коректно :)

@haki-1 haki-1 requested a review from code-hunger May 20, 2020 12:12
@haki-1 haki-1 changed the title fix access modifiers of evalAt and make TODOs in week 10 source fix Compose's domain and some other minor issues in week 11 source May 20, 2020
public:
ComposeSet(UniquePointer<RealFunc> a, UniquePointer<RealFunc> b) : a(a), b(b) {}

bool check(float x) const override { return a.ptr->getDomain().check(b.ptr->safeEval(x)); }
Copy link
Member

@code-hunger code-hunger May 20, 2020

Choose a reason for hiding this comment

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

Измислил си го! Ама почти… трябва да провериш също, че x е в домейна на b, иначе може да гръмне вътрешното извикване :)


UniquePointer(UniquePointer const& other) : ptr(new T(other.ptr)) {}
UniquePointer(UniquePointer const& other) : ptr(other.ptr->clone()) {}
Copy link
Member

Choose a reason for hiding this comment

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

Каква е идеята на тази промяна?

Copy link
Author

Choose a reason for hiding this comment

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

Параметърът T приема T = Set или T = RealFunc, така в предходната реализация имаше new израз на интерфейс, а на интерфейс не можем да създаваме инстанции. При новата реализация виртуалният метод clone изпълнява new израз и създава обект на клас-наследник, който не е абстрактен.

Copy link
Member

Choose a reason for hiding this comment

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

Хммм, да, прав си. То проблемът ни е, че изобщо дефинираме конструктор за копиране. По идея unique ptr е уникален, huh, и не би трябвало да предполага създаване на копия. Референцията на unique_ptr от стандартната библиотека дори го споменава изрично:

The class satisfies the requirements of MoveConstructible and MoveAssignable, but not the requirements of either CopyConstructible or CopyAssignable.

(виж, че дори няма дефиниран конструктор за копиране: https://en.cppreference.com/w/cpp/memory/unique_ptr/unique_ptr)

Всъщност първия проблем, който аз видях в това, е че UniquePtr става зависим от това върху T да е дефиниран подходящ метод clone(), а ние искаме UniquePtr да е възможно най-общ.

Най-добре е в такъв случай да махнем изцяло този ред и да предадем задачата за копиране на извикващата страна, а вместо това да направим конструктор от чист указател UniquePointer::UniquePointer(T*).

{
bool check(float x) const override
{
return fmod(x - M_PI / 2, M_PI) > eps || fmod(x - M_PI / 2, M_PI) < -eps;
Copy link
Member

Choose a reason for hiding this comment

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

Така е по-добре. А не можеш ли да комбинираш 2те условия в едно?

Насока: -e < x < e за положително епсилон е еквивалентно на |x| < e.

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