Skip to content

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

Open
haki-1 wants to merge 3 commits into
fmi-lab:masterfrom
haki-1:master
Open

fix Compose's domain and some other minor issues in week 11 source#1
haki-1 wants to merge 3 commits into
fmi-lab:masterfrom
haki-1:master

Conversation

@haki-1

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

Copy link
Copy Markdown

No description provided.

Comment thread 10.source.cpp

struct Product : public RealFunc
{
RealFunc *f, *g;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

Comment thread 10.source.cpp
std::cout << base->safeEval(3) << std::endl;
}

// problem with Compose's defined domain - example

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

Comment thread 10.source.cpp

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
Copy Markdown
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)); }

@code-hunger code-hunger May 20, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

UniquePointer(T const& other) : ptr(other.clone()) {}

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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