Hot answers tagged

14

Obviously, if you have any questions just ask. This is my first "big" project in OCaml, but I'd rather you didn't sugarcoat criticism. It looks like a very fun project, congratulations! :) I have a few comments, which all concerns the style of writing. This is an important topic, because a good style eases maintainability. The name of constructors: all ...


11

Nitpick private boolean exausthed = false; Pedantic, but this should be spelled exhausted. Class vs. Object private Set<Character> blankChars = new HashSet<Character>(); And then later in the constructor blankChars.add('\r'); blankChars.add('\n'); blankChars.add((char) 8); blankChars.add((char) 9); ...


5

I've never learned about lexers, so I can only answer 1-4 confidently. I hope this helps to answer some of your questions. I found your code quite easy to follow. I was able to read from top to bottom, and jumping to the other method definitions and back. It seems very well organized. Yes, having all the right sections of code nestled into their own ...


4

Is Janua really a valid month? If so, then you're fine, but I have a feeling that this would be more appropriate. DateMonthJanuary ::= 'January' | 'Jan' | '0'? '1' Which brings me to my next problem. It's case sensitive, right? So, 22-JAN-2015 wouldn't match. I believe it should, but I'm not familiar with Robol. Please correct me if I'm wrong. I've seen ...


4

I was about to submit this when I saw that you had visited #perl6. I'll leave this post as is because it is supposed to help others that might one day read this as well as you. TL;DR I recommend you: make sure you're ready for an immature compiler, ecosystem, doc, etc; use Rakudo Star and the Rakudo debugger; start small and take small steps, making each ...


4

Really accurate highlghting is a big challenge, and even if your implementation is not totally falsy it's very incomplete. First some obvious points, easy to correct: your PHP reserved words list lacks a number of words, such as (not exhaustively) global, const, if, else, switch, case, default, do, exit, break, continue try, catch, finally, ... you look ...


4

From looking at our code, I would assume that maintainability isn't important. If so, please add some comments so you understand the purpose of the members ... in six months. That said, there are already a few things you can do which I've learned are important for performance: Put the implementation of the constructor, destructor ... of ice_token in the ...


4

I'd expect to see many more tests of any program that addresses a problem as gnarly as parsing a C++ declaration. Choosing a couple I've recently had cause to write (on Stack Overflow), I would immediately add void (SENDER::*get_func())(double, double); (from Passing pointers to member function as returned values to QObject::connect()) constexpr std::...


3

haskell is Lazy, so should you be. You should consider using readDirectoryWithL over readDirectoryWith. That should have no heavy performance implications but still allow us to save a lot of memory. testTemplate is a bit confusing as a name. Why not outputTemplate? This IMO fits the usage better. You're not quite consistent as to when you use import ...


3

A potential solution to points #1 and #2 is to rework some of the recursive logic of the code. The basic premise is to rework the expression so it is more like: expression := [datum] [lparen expression rparen] This allows it to handle brackets by recursively calling itself until they are gone without using a bracket counter. I won't code the solution for ...


3

Look for corner cases This command is fragile, there are several ways in which it can malfunction: sed -i "/$LINE/d" ./$1 For example: If the dead code contains /, it will break the sed command, because / within /.../d must be escaped. It doesn't target accurately the line to remove. It removes all lines that match $LINE. If there are lines in the file ...


3

echo | grep is unwarranted. bash understands regular expressions ("$line" =~ regex), and can do simple substitutions: line=${line#[[:space:]]*\/\/} removes leading whitespaces , followed by the comment, just what we are after. Replacing the file while reading it looks suspiciously. I recommend to have a destination file, and copy valid lines (and don't copy ...


3

I cannot address your main concern about the undesirable behavior, and I am afraid nobody can: only you know what behavior is desirable. You may provide (failing) test cases, but that would jeopardize on-topikness of the question. Some random observation on the grammar follow: You disallow an empty statement_list, but you treat var_decl as a statement. It ...


3

This was fun to play with, so thanks for writing it and posting the question! After experimenting with it for a while, I tried to see if I could reduce the size of produce. I don't have all the intermediate steps to show, but I made the following transformations: I removed the type specifiers since the compiler was able to determine the types on its own. ...


3

Your hierarchy necessarily involves virtual methods, and therefore some kind of pointer. However, you have opted for a dangerous mix of raw pointers and shared pointers. The main problem is that a user of the Term class must deal with these pointers themselves, even though the pointers are merely an implementation detail of the Term class. It would therefore ...


3

I wanted to post an idea I had to address my concern over the brittleness of my unit test(s). I realised that since I am performing a string comparison, that I could use Regular Expressions. Then I can ignore parts of the parse tree string that are not interesting. So I have this as a potential solution: [Fact] public void ...


3

It doesn't look like this code is currently working. AFAIK Objective-C uses the normal C preprocessor. The preprocessor works on a token level, and does not know about stuff like method calls. It is entirely irrelevant that [ starts a method call or an array subscript, for the preprocessor it's just some token. So as a rough sketch: token TOP { ^^ \s* '...


2

I can only review the JS part, I am not sure that grammar reviews are part of CR. collapse else branches after a return do not make sense. I would propose map/join instead of reduce/concat Something likes this: function collapse(stuff) { if (typeof stuff === "string") return stuff; if (stuff instanceof Array) return stuff.map(...


2

I refactored for a while, largely replacing recursion with combinators. I think all these type aliases are unnecessary, I probably would have removed them if I had finished this. You didnt actually use the LHS in terminalsForRHS, except to pass it into recursive calls, so I took it out. module First where import Data.Map import Data.Maybe import Data.Either ...


2

associated_consts is stable since 1.20 (currently in beta). You don't need the attribute anymore. Other than that, you pass Clippy with no warnings. You should almost certainly #[derive(Debug)] on Row, and depending on usage, it may also make sense to also derive the other derivable traits that String impls. The biggest thing here is rightward drift. The ...


2

Let's start with the minor things: Personally, I like splitting my lexer grammar and parser grammar into two separate files. I understand that a lot of people don't like that though, so either way is fine. I split mine because the lexing and parsing are two separate logical steps that have nothing to do with each other. It also allows for some more powerful ...


2

You could learn how to use bison. It designed specifically for this. Boiler plate for flex/bison project: Eng.l %option c++ %{ #define IN_EXPR_LEXER #include "EngYac.hpp" %} SpaceChar [ \t] Space {SpaceChar}+ SpaceTerm {SpaceChar}|<<EOF>> FullStop {SpaceChar}\.{SpaceTerm} %% and {return yy::parser::...


2

I have no idea what you are trying to match with \\s+\\w+\\s+\\w+\\s*. If you are looking for a return type, then that won't do at all. It appears that you are trying to match the function body using \\{.*\\}. That won't work, because braces may be nested, and regular expressions cannot handle nesting. Modifier keywords can appear in any order, and you ...


2

I think you can get rid of the isinstance(..., list) checks by revising the grammar a bit. I renamed data_blocks to chunk because I kept mixing it up with data_block. A chunk is a depth_block followed by zero or more other_blocks. The visit_chunk function then knows the first child is a depth_block and the second child is a possibly empty list of ...


1

Here's a suggestion: For question #1 and #2, parse_expression is written in a way that matches the grammar, which also ensures the parens are balanced. If a '(' is matched, then the optional internal parts of an expression are parsed and then the ')'. There is a separate top level function call, parse(), that takes the expression to parse and makes sure ...


1

For the java spec there are already complete parsers based e.g. on JavaCC or Antlr. IMHO the safest way would be process a java file using one of those and extracting the methods from there. Processing using regular expressions is quite difficult, e.g. how about methods in comments. https://github.com/antlr/grammars-v4/tree/master/java8


1

This code looks pretty solid. It utilizes many ecmascript-6 features like arrow functions, default parameter values, const for any value that doesn't get re-assigned and let for values that can be re-assigned, etc. Comments could be added to help document the code. Some of the trivial functions could be simplified - e.g. function value(v) { return () =&...


1

Looking at int->roman ... It simply ignores digits that are too big. 10000 maps to ""! Surely better to repeat the biggest signifier as often as necessary. The logic for the one five ten units for digit is in your head. We can derive them from a list of the symbols in increasing order. And it's easier to wrap them as a single argument to digit. I've ...


1

Prefer std::make_shared<T>(...) over std::shared_ptr<T>(new T(...)). That's not only shorter, it's also potentially more efficient as it can coalesce the allocation for the counters and the payload. Otherwise, you need to make Term::~Term virtual (and explicitly defaulted), or polymorphic deallocation won't work. You can reuse a statically ...


Only top voted, non community-wiki answers of a minimum length are eligible