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

[WIP] Big O attribute notation #4965

Open
wants to merge 2 commits into
base: master
from

Conversation

@andralex
Copy link
Member

commented Dec 18, 2016

Associated article: http://erdani.com/d/bigo.html

@andralex andralex force-pushed the andralex:bigo branch from 3c24b21 to 839fb6d Dec 18, 2016

@codecov-io

This comment has been minimized.

Copy link

commented Dec 18, 2016

Current coverage is 80.67% (diff: 0.00%)

Merging #4965 into master will decrease coverage by 8.72%

@@             master      #4965   diff @@
==========================================
  Files           124        124           
  Lines         77033      77012     -21   
  Methods           0          0           
  Messages          0          0           
  Branches          0          0           
==========================================
- Hits          68871      62132   -6739   
- Misses         8162      14880   +6718   
  Partials          0          0           

Powered by Codecov. Last update 9965433...3c24b21

@atilaneves

This comment has been minimized.

Copy link
Contributor

commented Dec 19, 2016

No @safe on the unittest blocks.

struct BigO
{
import std.typecons : Tuple;
import std.algorithm : cartesianProduct, map, sum;

This comment has been minimized.

Copy link
@JackStouffer

JackStouffer Dec 21, 2016

Member

This should be moved into the functions were they are used

import std.typecons : Tuple;
import std.algorithm : cartesianProduct, map, sum;
import std.array : array, empty, front, popFront;
import std.range : chain;

This comment has been minimized.

Copy link
@JackStouffer

JackStouffer Dec 21, 2016

Member

same here

string toString() const @safe
{
if (terms.empty) return "O(1)";
string result = "O(";

This comment has been minimized.

Copy link
@JackStouffer

JackStouffer Dec 21, 2016

Member

I would use std.array.appender here.

Plus, since you know the length of terms and it's sub arrays, you could create a pretty good guess for the size needed, and call appender.reserve at the start of the function.

result ~= " * ";
}
result ~= "log(n";
if (f.id) result ~= f.id.to!string;

This comment has been minimized.

Copy link
@JackStouffer

JackStouffer Dec 22, 2016

Member

It's a micro optimization to be sure, but if you use appender in this function, then instead of

result ~= f.id.to!string;

which allocates a temporary array then throws it away, you can do

result.put(f.id.toChars);

which will only use appender's internal buffer

@JackStouffer

This comment has been minimized.

Copy link
Member

commented Mar 7, 2018

I think, for now, the best place for this would be dub.

@burner burner referenced this pull request May 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.