Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(51)

Issue 239180043: 4 - Danila Chesnokov - 2

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 10 months ago by ChesnokovDS
Modified:
8 years, 10 months ago
Visibility:
Public.

Patch Set 1 #

Total comments: 57

Patch Set 2 : second iter #

Total comments: 40

Patch Set 3 : third iteration #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+266 lines, -0 lines) Patch
M minimal_string.cpp View 1 chunk +266 lines, -0 lines 2 comments Download

Messages

Total messages: 8
ChesnokovDS
Первая посылка.
8 years, 10 months ago (2015-05-22 09:09:12 UTC) #1
algorithms.shad.nsk
https://codereview.appspot.com/239180043/diff/1/minimal_string.cpp File minimal_string.cpp (right): https://codereview.appspot.com/239180043/diff/1/minimal_string.cpp#newcode27 minimal_string.cpp:27: Rule() {} Лучше NT проинициализировать на всякий случай какой-нибудь ...
8 years, 10 months ago (2015-05-23 11:22:49 UTC) #2
ChesnokovDS
Замечания исправил, за исключением пожалуй https://codereview.appspot.com/239180043/diff/1/minimal_string.cpp#newcode143 честно говоря не понял что требуется. В данном участке ...
8 years, 10 months ago (2015-05-26 18:56:36 UTC) #3
ChesnokovDS
Замечания исправил, за исключением пожалуй https://codereview.appspot.com/239180043/diff/1/minimal_string.cpp#newcode143 честно говоря не понял что требуется. В данном участке ...
8 years, 10 months ago (2015-05-26 18:56:37 UTC) #4
chesnokovds_gmail.com
Кстати, это неоткалиброваность тестов или парсер Ирли так хорош, что проходит тесты, не более чем ...
8 years, 10 months ago (2015-05-26 19:01:50 UTC) #5
algorithms.shad.nsk
https://codereview.appspot.com/239180043/diff/20001/minimal_string.cpp File minimal_string.cpp (right): https://codereview.appspot.com/239180043/diff/20001/minimal_string.cpp#newcode20 minimal_string.cpp:20: typedef size_t counter; Зачем этот typedef? https://codereview.appspot.com/239180043/diff/20001/minimal_string.cpp#newcode38 minimal_string.cpp:38: explicit ...
8 years, 10 months ago (2015-05-27 05:34:42 UTC) #6
ChesnokovDS
https://codereview.appspot.com/239180043/diff/20001/minimal_string.cpp File minimal_string.cpp (right): https://codereview.appspot.com/239180043/diff/20001/minimal_string.cpp#newcode20 minimal_string.cpp:20: typedef size_t counter; On 2015/05/27 05:34:41, algorithms.shad.nsk wrote: > ...
8 years, 10 months ago (2015-05-29 12:58:36 UTC) #7
algorithms.shad.nsk
8 years, 10 months ago (2015-05-30 18:02:42 UTC) #8
Ладно, засчитываю.

https://codereview.appspot.com/239180043/diff/40001/minimal_string.cpp
File minimal_string.cpp (right):

https://codereview.appspot.com/239180043/diff/40001/minimal_string.cpp#newcode48
minimal_string.cpp:48: struct ImportRule : Rule {
Ну что за уродство? =)
Почему нельзя сделать просто метод Rule::Import(string)?
Откуда вообще взялась идея про наследование структур? =)

https://codereview.appspot.com/239180043/diff/40001/minimal_string.cpp#newcod...
minimal_string.cpp:119: if ((*it)->NT == new_NT) {
Вы вроде завели псевдоним, а всё равно его не используете: почему-то здесь мир
скобочек и звёздочек остался.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b