Skip to content

ocl plugin#62

Open
SkyForce wants to merge 40 commits intoyurii-litvinov:masterfrom
SkyForce:master
Open

ocl plugin#62
SkyForce wants to merge 40 commits intoyurii-litvinov:masterfrom
SkyForce:master

Conversation

@SkyForce
Copy link
Copy Markdown

No description provided.

Comment thread src/WpfEditor/WpfEditor.csproj Outdated
<Prefer32Bit>true</Prefer32Bit>
</PropertyGroup>
<ItemGroup>
<Reference Include="Antlr4.Runtime.Standard, Version=4.7.1.0, Culture=neutral, PublicKeyToken=e78b2c5abd1fcb3f, processorArchitecture=MSIL">
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

WpfEditor точно должен зависеть от ANTLR?

<Generator>ResXFileCodeGenerator</Generator>
<LastGenOutput>Resources.Designer.cs</LastGenOutput>
</EmbeddedResource>
<None Include="packages.config" />
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Мы используем paket

@@ -0,0 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Этот файл не нужен, управление зависимостями делается Paket-ом

Comment thread src/plugins/OclPlugin/FunctionDef.cs Outdated
public List<string> param;
public OclParser.ExpressionContext context;
}
}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Тут с отступами какая-то беда

Comment thread src/plugins/OclPlugin/FunctionDef.cs Outdated
{
public string name;
public List<string> param;
public OclParser.ExpressionContext context;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Публичные поля нельзя

Comment thread src/plugins/OclPlugin/OclPrinter.cs Outdated
}
else if (context.pathName().GetText() == "forAll")
{
Dictionary<string, object> st = new Dictionary<string, object>();
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Тут тоже надо var. И вообще, посмотрите по коду, кажется, ещё много где var был бы уместен.

Comment thread src/plugins/OclPlugin/OclPrinter.cs Outdated
else if (this.Res is LinkedList<object>)
{
ar = new LinkedList<object>();
}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Есть же switch с pattern matching-ом

Comment thread src/plugins/OclPlugin/OclPrinter.cs Outdated
}
else if (context.pathName().GetText() == "select")
{
ICollection<object> ar = null;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

На всякий случай ещё раз настоятельно рекомендую убрать все сокращения :) Это ж не ассемблер.

Comment thread src/plugins/OclPlugin/OclPrinter.cs Outdated
}

return this.VisitPathName(context.pathName());
}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Стоит разбить на методы поменьше

Comment thread src/plugins/OclPlugin/OclPrinter.cs Outdated
using EditorPluginInterfaces;
using Repo;

internal class OclPrinter : OclBaseVisitor<bool>
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

А почему он Printer? Мне кажется, он больше Interpreter, чем Printer

Comment thread OclPlugin.Tests/UnitTest1.cs Outdated
@@ -0,0 +1,41 @@
using System.Collections.Generic;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

UnitTest1.cs как имя файла, мягко говоря, не очень

Comment thread OclPlugin.Tests/UnitTest1.cs Outdated
}

[Test]
public void Test1()
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Test1 :(

Comment thread OclPlugin.Tests/UnitTest1.cs Outdated
public void Test1()
{
var model = new WpfControlsLib.Model.Model();
var repo = model.Repo;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

А можно же просто репозиторий создать, через RepoFactory. Зависеть от всего GUI только для того, чтобы получить репозиторий, архитектурно неправильно

public override Result Divide(Result res)
{
throw new NotImplementedException();
}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Тут лучше везде expression-bodied-синтаксис использовать,

public override Result Divide(Result res) => throw new NotImplementedException();

А конкретно эти методы я бы вообще в базовом классе реализовал как бросающие NotImplementedException, чтобы в потомках не писать одно и то же дважды

public abstract Result Multiply(Result res);

public abstract Result Divide(Result res);
}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Абстрактный класс, к уоторого нет полей и все методы абстрактные --- это интерфейс

{
private readonly ArrayList<Dictionary<string, Result>> vars;
private readonly OclCalculator calculator;
private readonly Dictionary<string, FunctionDefinition> funcs;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

functions?

return filteredElements;
}

Dictionary<string, Result> stack = new Dictionary<string, Result>();
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

var


this.vars.RemoveAt(this.vars.Count - 1);
return filteredElements;
}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Этот гигантский if можно как-то более поООПшному сделать, мне кажется. context.pathName().GetText() используется как тэг типа, а тела if-ов --- как виртуальные методы, а всё это вместе --- симуляция lookup-а в VMT вручную.

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