Overloading == for better readability I was working on a project recently that had a good amount of business logic associated with status codes.   To better understand the flow of logic in the domain layer, I created an enum representation of the statuses and overloaded the equality operator in the status domain model to check against the enum.

Is this good practice, or does this an unnecessary abstraction?  Also seeing if I should implement the GetHashCode() and Equal() methods or perhaps just make them just return

() => throw new InvalidOperationException();

Implementation

public class StatusDm
{

    public StatusDm() { }
    public StatusDm(Models.Database.Status status)
    {
        Id = status.Id;
        Name = status.Name;
    }
    public int Id { get; set; }
    public string Name { get; set; }

    public void DatabaseTransfer(ref Models.Database.Status status)
    {
        status.Name = Name;
    }
    public static bool operator ==(StatusDm domainStatus, Models.Constants.StatusEnum status)
    {
        return domainStatus?.Id == (int)status;
    }

    public static bool operator !=(StatusDm domainStatus, Models.Constants.StatusEnum status)
    {
        return !(domainStatus?.Id == (int)status);
    }
}

Usage

if(individual.StatusModel == StatusEnum.Submitted || individual.StatusModel == StatusEnum.Disqualified)
{
    ...
}

Appreciate any feedback or critic. Thanks!

Edit:  There was a question in regards to how it would normally be done in this application.  Here was the evolution of the usage statement

First Implementation

if (storedIndividual.Status == (int)IndividualStatus.InProgress &&
    (individual.StatusModel.Id == (int)IndividualStatus.Submitted ||
        individual.StatusModel.Id == (int)IndividualStatus.Disqualified))

{
    SetFinishedIndividualFormValues(ref individual);
    if (individual.StatusModel.Id == (int)IndividualStatus.Submitted)
    {
        SendSubmittedFileEmail(individual);
    }
}
else if (individual.StatusModel.Id != (int)IndividualStatus.Disqualified)
{
    SendStatusChange(individual);
}

Revised Implementation

private void HandleStatusChanges(Models.Database.Individual storedIndividual, ref Models.Domain.IndividualDm individual)
{
    if (storedIndividual.Status == (int)IndividualStatus.InProgress &&
        (individual.Status == IndividualStatus.Submitted ||
            individual.Status == IndividualStatus.Disqualified))
    {
        SetFinishedIndividualFormValues(ref individual);
        if (individual.Status == IndividualStatus.Submitted)
        {
            SendSubmittedFileEmail(individual);
        }
    }
    else if (individual.Status != IndividualStatus.Disqualified)
    {
        SendStatusChange(individual);
    }
}

It is difficult to judge without full code, but while keeping things mutable (please use a more descriptive names, a future you will thank you):

public class StatusModel
{
    public IndividualStatus Status { get; set; }
    public int StatusId { get => (int)Status; set => Status = (IndividualStatus)value; }
    public bool InProgressStatus => Status == IndividualStatus.InProgress;
    public bool SubmittedStatus => Status == IndividualStatus.Submitted;
    public bool DisqualifiedStatus => Status == IndividualStatus.Disqualified;
    public bool DisqualifiedOrSubmittedStatus => DisqualifiedStatus || SubmittedStatus;
    public bool JustFinished(StatusModel stored) =>
          stored.InProgressStatus && DisqualifiedOrSubmittedStatus;
    public bool JustSubmitted(StatusModel stored) =>
        JustFinished(stored) && SubmittedStatus;
    public bool StillInProgress(StatusModel stored) =>
        !JustFinished(stored) && !DisqualifiedStatus;
}

As a next step you could also define an implicit conversion operators from/to int/enum types and make this class immutable.

public class StatusModel
{
    public static implicit operator StatusModel(IndividualStatus status) => new StatusModel(status);
    public static implicit operator IndividualStatus(StatusModel model) => model.Status;
    public static implicit operator StatusModel(int id) => (IndividualStatus)id;
    public static implicit operator int(StatusModel model) => (int)model.Status;
    StatusModel(IndividualStatus status) => Status = status;
    public IndividualStatus Status { get; set; }
    public bool InProgressStatus => Status == IndividualStatus.InProgress;
    public bool SubmittedStatus => Status == IndividualStatus.Submitted;
    public bool DisqualifiedStatus => Status == IndividualStatus.Disqualified;
    public bool DisqualifiedOrSubmittedStatus => DisqualifiedStatus || SubmittedStatus;
    public bool JustFinished(StatusModel stored) =>
          stored.InProgressStatus && DisqualifiedOrSubmittedStatus;
    public bool JustSubmitted(StatusModel stored) =>
        JustFinished(stored) && SubmittedStatus;
    public bool StillInProgress(StatusModel stored) =>
        !JustFinished(stored) && !DisqualifiedStatus;
}

It is a bunch of code (C# is a ridiculously verbose language), but it would be very easy to have more logic here, adding anything status related would be a very cheap operation.

private void HandleStatusChanges(
    Models.Database.Individual storedIndividual,
    ref Models.Domain.IndividualDm individual)
{
    if (individual.Status.JustFinished(storedIndividual.Status))
        SetFinishedIndividualFormValues(ref individual);
    if (individual.Status.JustSubmitted(storedIndividual.Status))
        SendSubmittedFileEmail(individual);
    if (individual.Status.StillInProgress(storedIndividual.Status))
        SendStatusChange(individual);
}

I would also define and associate custom JsonConverter and TypeConverter with StatusModel, so it will look like a status integer ID for asp.net (see an example here).

P.S. Generally speaking, data entity types should never be visible in the business core:
https://blog.cleancoder.com/uncle-bob/2016/01/04/ALittleArchitecture.html


Generate sequence in Linq I want to generate a sequence using Linq that goes from 10 to 100 with a step size of 10. The sequence also must contain a custom value (in the correct order).

This is what I have now, and I'm wondering if there is a smarter way of doing it using Linq expressions.

var pageSize = 25; //number that must be in sequnce if not already, can be anything.
var items = Enumerable.Range(10, 100)     //Generate range
           .Where(x => x % 10 == 0)       //Take every 10th item
           .Concat(new[] { pageSize })    //Add the custom number
           .Distinct()                    //If the custom number was already in there, remove it
           .OrderBy(x => x);              //Sort the sequence

Result:

10
20
25
30
40
50
60
70
80
90
100

Size of initial list in the following code is less:

var x = Enumerable.Range(1, 10)
                  .Select(x_ => x_ * 10)
                  .Concat(new[] {25})
                  .Distinct()
                  .OrderBy(x_ => x_)
                  .ToList();

Otherwise your original code is perfect.


Implementing (+++) to Join JoinList's Related to this question, I wrote the (+++) function to join 2 JoinList's:

(+++) :: Monoid m => JoinList m a -> JoinList m a -> JoinList m a
(+++) Empty               y                    = y
(+++) x                   Empty                = x
(+++) left@(Single _ _)   right@(Single _ _)   = Append (tag $ Append mempty left right) left right
(+++) left                right                = Append (tag left `mappend` tag right) left right

Calling (+++) on jl1 and jl2 results in:

jl1 :: JoinList (Product Integer) String
jl1 = Append (Product 100) (Single (Product 25) "foo") (Single (Product 4) "bar")

jl2 :: JoinList (Product Integer) String
jl2 = Append (Product 50) (Single (Product 25) "bippy") (Single (Product 2) "baz")

Testing

*JoinList> (+++) jl1 jl2
Append (Product {getProduct = 5000})
          (Append (Product {getProduct = 100})
               (Single (Product {getProduct = 25}) "foo")
               (Single (Product {getProduct = 4}) "bar"))
          (Append (Product {getProduct = 50})
               (Single (Product {getProduct = 25}) "bippy")
               (Single (Product {getProduct = 2}) "baz"))

(I didn't know about the other JoinList questions when I was answering the other one one you asked.)

The case

(+++) left@(Single _ _)   right@(Single _ _)   =
  Append (tag $ Append mempty left right) left right

is redundant. There is no need to distinguish this case, also there is no need to call tag on a temporarily created Append value. Other than that, it looks good.

Just a remark, using Product with JoinList probably isn't very meaningful, monoids that provide useful information (depending on the use-case) would be Sum, Min or Max.

For testing, I'd suggest you to study QuickCheck. It allows you to define properties of your operations and automatically test them for various inputs. It's important to define good properties, in particular they should be distinct from the implementation of your operations. For example, you might want to test: For any JoinLists xs and ys

if i < sizeJ xs then indexJ i (xs +++ ys) === indexJ i xs
if i >= sizeJ xs then indexJ i (xs +++ ys) === indexJ (i - sizeJ xs) ys

etc.


Outputs all substrings that are palindromes in a given string, in alphabetical order without repitition I'm currently trying to find a way to get my program to run more efficiently. If anyone has any ideas on how to optimize my program, i would appreciate it a lot. Also, the output has to display in alphabetic order without any duplicates.

package Palindrome;
import java.io.*;
import java.util.*;
public class Palindrome {

public static void main(String[] args)throws IOException{
    Scanner scan;
    scan = new Scanner(System.in);
    while (scan.hasNext()) {

subPal(scan.next());
        }
    if (scan != null) {
        scan.close();
    }}

private static void subPal(String str) {
    String s1 = "";
    int N = str.length();
    ArrayList<String> palindromeArray = new ArrayList<String>();
    for (int i = 2; i <= N; i++) {
        for (int j = 0; j <= N; j++) {
            int k = i + j - 1;
            if (k >= N)
            continue;
            s1 = str.substring(j, i + j);
            if (s1.equals(new StringBuilder(s1).reverse().toString())&&    !palindromeArray.contains(s1)) {
            palindromeArray.add(s1);
        }
    }

}
Collections.sort(palindromeArray);
for (String s : palindromeArray)
    System.out.println(s );

}

}

Performance

These are slow operations that can be replaced with faster ones:

s1 = str.substring(j, i + j); creates new strings. A small improvement would be to use .charAt calls to access elements of str without creating copies. Another small improvement on top of that would be to convert str to a char[] using str.toCharArray() and use array indexes to access elements of the string.
s1.equals(new StringBuilder(s1).reverse().toString()) is a lazy way to check if a string is a palindrome. It creates a new string, and then iterates over values to check if palindrome. It would be a lot better to implement a function for that, comparing pairs of letters from the beginning and end, closing in toward the center.
!palindromeArray.contains(s1) is an \$O(n)\$ operation that can be easily turned into \$O(1)\$ by using a Set instead of an array.
The slowest part of all is the main algorithm of enumerating possible palindromes. The nested loop basically checks all possible substring. That's not necessary. A more clever approach would be, for each position, center on the position, and keep going outward as long as the start and end letters still form a palindrome. This way you can drastically reduce the number of comparisons made in the process of checking for palindromes.

Coding style

The coding style of your post is extremely poor.

Indentation is inconsistent, messy
Names are poor. palindromeArray is not an "array", it's a list. And you could just call it palindromes, naturally. subPal is cryptic at best. How about printUniquePalindromes?
It's recommended to use braces with single-statement if and for statements too.

Other issues

The scan != null condition is always true, so the if statement is unnecessary.

The initializer in String s1 = "" is redundant, String s1 would be enough


C++ vector that uses expression templates technique to increase performance of mathematical expressions I've been reading Item 6 from Scott Meyers' Effective Modern C++ and noticed that he mentioned a technique called expression templates. I've decided to give it a try and implemented a vector that supports addition and subtraction:

#include <iostream>
#include <vector>

template<typename T>
class Vec
{
public:
    std::vector<T> data;

    typedef typename std::vector<T>::size_type size_type;

    Vec(size_type size): data(size)
    {
    }

    Vec(const std::initializer_list<T>& elements): data(elements.size())
    {
        size_type i = 0;
        for (const auto& el: elements)
        {
            data[i++] = el;
        }
    }

    template<typename VecOperation>
    Vec(const VecOperation& vo): data(vo.t2.data.size())
    {
        for (size_type i = 0; i < data.size(); ++i)
        {
            data[i] = vo[i];
        }
    }

    T operator[](size_type i) const
    {
        return data[i];
    }
};

template<typename T1, typename T2>
struct VecSum
{
    const T1& t1;
    const T2& t2;
    auto operator[](typename T2::size_type i) const
    {
        return t1[i] + t2[i];
    }
};

template<typename T1, typename T2>
struct VecDiff
{
    const T1& t1;
    const T2& t2;
    auto operator[](typename T2::size_type i) const
    {
        return t1[i] - t2[i];
    }
};

template<typename T1, typename T2>
auto operator+(const T1& t1, const T2& t2)
{
    return VecSum<T1, T2>{t1, t2};
}

template<typename T1, typename T2>
auto operator-(const T1& t1, const T2& t2)
{
    return VecDiff<T1, T2>{t1, t2};
}

int main()
{
    Vec<int> v1{1, 2, 3, 4, 5};
    Vec<int> v2{6, 7, 8, 9, 11};
    Vec<int> v3{3, 5, 2, 0, 17};

    Vec<int> v4 = v1+v2-v3;

    for (const auto& x: v4.data)
    {
        std::cout << x << ", ";
    }
    std::cout << std::endl;

    return 0;
}

The main advantage of this solution is that in the line Vec<int> v4 = v1+v2-v3; no additional temporaries of type Vec are created, which increases performance.

I'd be grateful if someone could point potential drawbacks and possible improvements of this code.

Generally speaking, it's pretty good. However, there are still a few things that I would have done differently:

typedef becomes kind of ugly now that we have type aliases. Always using type aliases generally tends to produce more readable and more consistent code:

using size_type = typename std::vector<T>::size_type;

Loop only when you have to. In your std::initializer_list constructor, you don't need to loop; you can use std::vector's constructor taking a pair of iterators instead:

Vec(const std::initializer_list<T>& elements):
    data(std::begin(elements), std::end(elements))
{
}

By the way std::initializer_list tends to be no more than a pair of pointers. You can take it directly by value instead of taking it by const reference.
In operator+ and operator-, I would have placed the return type into the function signature and then used the list initialization syntax in the return statement:

template<typename T1, typename T2>
auto operator+(const T1& t1, const T2& t2)
    -> VecSum<T1, T2>
{
    return { t1, t2 };
}

Whether it is better or not is debatable but when the return type is fixed, rather meaningful and arguably simple, I prefer to expose it in the signature.


PHP Simple OOP Class I am still learning OOP and I need a little help, I don't really know how to properly make this not look so terrible.

Any thoughts?

<?php

class Noti {

    public static $app;
    public static $user;
    public static $url;
    public static $result;
    public static $data;
    public static $title;
    public static $text;

    public static function status($title = null, $text = null) {

        // Error check
        if(empty($title)) return false;
        if(empty($text)) return false;

        // Set
        self::$title = $title;
        self::$text = $text;

        // Get the NOTI config
        self::$app  = Config::get('noti.app_id');
        self::$user = Config::get('noti.user_token');
        self::$url  = Config::get('noti.url');

        // Format
        self::$data = self::format();

        // Url
        $ch = curl_init(self::$url.'add'); // Append add to url for status update
        curl_setopt($ch, CURLOPT_POST, 1);
        curl_setopt($ch, CURLOPT_POSTFIELDS, self::$data);

        self::$result = curl_exec($ch);

        // Close
        curl_close($ch);

        return self::$result;
    }

    private static function format() {
        $data = array(
            'app' => self::$app,
            'user' => self::$user,
            'notification[title]' => self::$title,
            'notification[text]' => self::$text
        );

        return http_build_query($data);
    }

}

This is not OOP.

Just using class does not make your code object oriented.  Writing OO code is about encapsulating the state and behavior into objects.

See this answer which explains why public properties and static are bad.

Static is Already Hurting You

Your code relies on Config::get.  Anyone who wants to reuse this class will need to have a class named Config with a method named get.  This is what your class depends on.  If you try to unit test this class you will find it difficult to test it as a unit.  It is already hard-coded to rely on the Config class.

Dependency Injection / Inversion Of Control

To remove the tight coupling to your Config object you should pass in the dependencies to the constructor of the object:

class Noti
{
    protected $app;
    protected $config;

    public function __construct(Config $config)
    {
        $this->config = $config;
    }

    public function getStatus()
    {
        $this->app = $this->config->get('noti.app_id');
        // etc.
    }
}

Note: I would use docblock comments above to explain all of the methods and properties.

The tight coupling to the Config class's get method has been modified.  From above we see that any Config object or derived class (We type-hinted for Config in the constructor) can be passed into the Noti class.  This could be further decoupled by Type-Hinting for an interface that specifies the contract for the Config object implementation.

The testing is now simple as a mock object can be passed into the constructor and there are no hard-coded dependencies left in the code.

Method Names

Method names should be descriptive.  Consider using verb+noun to describe your methods:

getStatus // instead of status
getQuery  // instead of format

Now we know that we are returning the status, and formatting a URL.  Note: As mseancole points out you are doing too much in your status method.


Defusing a bomb using a state machine This code is a solution for /r/daily_programmer #293.

The idea of this challenge is, you are given input in the form of colored wires:

red wire
green wire
blue wire
...

and you are then to determine if cutting the input wires in the order they are listed will cause the "bomb" to detonate or defuse.

What causes the detonation is determined by the previous wire cut.  The challenge lists a pre-determined set of colored wires for which the previous wire is not allowed to be followed by a cut of that color or the bomb will detonate.

#include <vector>
#include <fstream>
#include <iostream>

using namespace std;

#define ERROR(msg)           \
    do {                     \
        cout << msg;         \
        exit(1);             \
    } while(1)               \

/*
 * WHITE  :: !WHITE,  !BLACK
 * RED    :: !WHITE,  !RED,    !BLACK  !ORANGE, !PURPLE
 * BLACK  :: !WHITE,  !GREEN,  !ORANGE
 * ORANGE :: !WHITE,  !ORANGE, !GREEN, !PURPLE
 * GREEN  :: !RED,    !BLACK,  !GREEN, !PURPLE
 * PURPLE :: !PURPLE, !GREEN,  !WHITE, !ORANGE
 */

string color_names[] = { "white",
                         "red",
                         "black",
                         "orange",
                         "green",
                         "purple" };

#define WHITE   1 << 0
#define RED     1 << 1
#define BLACK   1 << 2
#define ORANGE  1 << 3
#define GREEN   1 << 4
#define PURPLE  1 << 5

#define COLORS       6

int color_values[] { WHITE,
                     RED,
                     BLACK,
                     ORANGE,
                     GREEN,
                     PURPLE };

int conflicts[COLORS] = { WHITE | BLACK,
                          WHITE | RED | BLACK | ORANGE | PURPLE,
                          WHITE | GREEN | ORANGE,
                          WHITE | ORANGE | GREEN | PURPLE,
                          RED | BLACK | GREEN | PURPLE,
                          PURPLE | GREEN | WHITE | ORANGE };

class Wire {

    private:
        int color_value;
        int color_conflicts;

    public:
        Wire();
        Wire(string color);
        string color_name;
        int is_conflicted(Wire wire);
};

Wire::Wire(void)
{}

Wire::Wire(string color)
{
    for (int i=0; i < COLORS; i++)
        if (!color.compare(color_names[i])) {
            color_name = color_names[i];
            color_value = color_values[i];
            color_conflicts = conflicts[i];
        }
}

int Wire::is_conflicted(Wire wire)
{
    return color_conflicts & wire.color_value;
}

class Bomb {

    private:
        Wire current_wire;
        int explode(void);
        vector<Wire> previous_cuts;

    public:
        Bomb(string initial_wire);
        int cut_wire(string wire);
        vector<Wire> get_previous_cuts();
};

Bomb::Bomb(string initial_wire)
{
    string wire(initial_wire);
    Wire current_wire(wire);
    previous_cuts.push_back(current_wire);
}

int Bomb::cut_wire(string wire)
{
    Wire new_wire(wire);
    previous_cuts.push_back(new_wire);

    if (current_wire.is_conflicted(new_wire))
        return explode();
    current_wire = new_wire;
    return 0;
}

int Bomb::explode(void)
{
    cout << "Bomb explodes\n";
    return -1;
}

int main(int argc, char *argv[])
{
    if (argc < 2)
        ERROR("Error Usage: ./bomb_defuse_challenge <input>\n");

    string bomb_fname(argv[1]);
    ifstream bomb_fh(bomb_fname);

    if (!bomb_fh.is_open())
        ERROR("Error reading bomb input\n");

    string input_wire_color;
    getline(bomb_fh, input_wire_color);
    Bomb bomb(input_wire_color);

    while (getline(bomb_fh, input_wire_color)) {
        if (bomb.cut_wire(input_wire_color) < 0)
            return 0;
    }

    cout << "Bomb defused\n";

    return 0;
}

Two things stand out:

In your Bomb constructor, you do this:

Wire current_wire(wire);

This is creating a locally scoped variable current_wire and setting its value to wire, it's not updating the member variable of the class.
You are keeping track of more information than you need to.  You only need know the previous wire to know if the current wire is going to cause an explosion.  You are however keeping track of every wire in previous_cuts, although you're not using the value anyway.  Generally, you're better off not having redundant code, so if you're not going to use it, get rid of it.


Listing WordPress posts with thumbnail images I am using the following code poor form. I am interested in how I can optimise this and use the correct methods. I understand the DRY concept and will look to employ that throughout my code. However, this works and its pretty quick too, even with large result sets. I would like to learn and improve this code.

   <?php

global $current_user;
get_currentuserinfo();

$user_id = $current_user->ID;
$blogUrl = get_site_url();
global $wpdb;
$DBquery = "
SELECT *
FROM saved_collection
WHERE userid = '".$user_id."'
";

$result = $wpdb->get_results($DBquery);

$ids = array();
foreach($result as $row)
{

$collectionID = $row->id;
$collection_name = $row->collection_name;
$collectionURL = 'Programme Link';

echo '
';
echo '
' . $collection_name . '
';
echo  $collectionURL;
echo '

';
$cleandata = unserialize($row->collection_data);

  foreach($cleandata as $data)
  {
          $ids[] = $data['ExerciseID'];
        // START OF QUERY TO GET THE FIRST IMAGE FROM EACH EXERCISE
          global $wpdb;
            $query = "
            SELECT *
            FROM imagemap
            WHERE exercise = '".$data['ExerciseID']."'
            LIMIT 1
            ";

            $result = $wpdb->get_results($query);

            foreach($result as $row)

                 {
                  echo '
';
                // START OF QUERY TO GET NAME OF EACH IMAGE
                    global $wpdb;
                    $exerciseTitleQuery = "
                    SELECT post_title
                    FROM wp_posts
                    WHERE id = '".$data['ExerciseID']."'
                    LIMIT 1
                    ";
                    $exerciseTitleQueryResult = $wpdb->get_results($exerciseTitleQuery);
                    foreach ($exerciseTitleQueryResult as $returned_post_title)
                        {echo '
';
                             echo '
' .$returned_post_title->post_title. '
';
                             echo '
';
                        }
                // END OF QUERY TO GET NAME OF EACH IMAGE

                echo "id. ".jpg />";
                    //echo '
id. '.jpg);">
';

                  echo '
';
                  // end of div imageGridImages
         }
  }

echo '

';
echo '
';

}
?>

One of the nested loops can be eliminated, by joining these two queries:

$query = "
SELECT *
FROM imagemap
WHERE exercise = '".$data['ExerciseID']."'
LIMIT 1
";

$result = $wpdb->get_results($query);

foreach($result as $row) {
    echo '<div class="imageGridImages imageGridImagesLarge">';
    global $wpdb;
    $exerciseTitleQuery = "
    SELECT post_title
    FROM wp_posts
    WHERE id = '".$data['ExerciseID']."'
    LIMIT 1
    ";

This code is inefficient, because for every row in the first query, you run another query. For example if there are 10 rows in the first query, you'll be running 11 queries. Queries can be expensive, it's better to minimize them as much as possible, for example by selecting multiple rows in bulk instead of doing it one by one. Since in both the outer and the inner queries the parameter is the same, $data['ExerciseID'], you should be able to use a JOIN instead in a single query.

Another problem you have in the first query is that SELECT * is not recommended. You should specify exactly the columns you need, and their order. When you use SELECT *, you might be selecting too much, downloading more data than you really need, and if the order columns change in the database or new columns are added, you could have nasty bugs.


Anchor Text String Transformation in Swift I need to convert a string using these rules:

Downcase the string
Replace spaces, a blacklist of "invalid" chars, and non-ascii letters (like é) with -
Replace repeated hyphens (ie --) with one hyphen (-)

For example, Is my résumé good enough? should convert to is-my-r-sum-good-enough-.

This Swift function converts the string. I'm looking for help making it shorter. Also, if there's a way to avoid import Foundation (by replacing containsString and stringByReplacingOccurrencesOfString), I'd like to do that. I know I could use an NSRegularExpression but I don't want to use Foundation classes.

Here's what I have so far:

extension Character {
    static let invalidSet = Set(" -&+$,/:;=?@\"#{}|^~[]`\\*()%.!'".characters)

    var invalid: Bool {
        return Character.invalidSet.contains(self)
    }

    var isAscii: Bool {
        guard let number = String(self).utf16.first else { return false }
        return (65..<127) ~= number
    }
}

func anchor(header: String) -> String {
    var output = String(header.lowercaseString.characters.map {
        $0.invalid || !$0.isAscii ? "-" : $0
    })

    while output.containsString("--") {
        output = output.stringByReplacingOccurrencesOfString("--", withString: "-")
    }

    return output
}

Usage:

let input = "Is my résumé good enough?"
let expectedResult = "is-my-r-sum-good-enough-"

print(anchor(input)) //prints is-my-r-sum-good-enough-
print(anchor(input) == expectedResult ? "" : "") //prints

We should always be thinking about how to write our code in a generic way.  Otherwise, what happens if we want something very similar to what we've just written, but slightly different in some minor way?  Well, it usually results in a lot of copy & pasting.  Let's write some code that will allow us to apply the rules you want to apply, but in a more generic way.

I want to start with a skeleton that looks something like this:

struct StringFormatter {
    enum CaseRule {
        case None, UppercaseOnly, ConvertToUpper, LowercaseOnly, ConvertToLower
    }

    enum AsciiRule {
        case None, AsciiOnly
    }

    var blacklistCharacters = Set<Character>()
    var replacementCharacter = ""
    var caseRule = CaseRule.None
    var asciiRule = AsciiRule.None

    func stringByApplyingFormatting(toString string: String) -> String {
        // TODO: Implement actual formatting logic
        return ""
    }
}

Now we've got a reusable structure for applying this sort of formatting to our strings.  Importantly, despite there being no logic in it yet, we have implemented the method we'd be calling to apply the formatting, so we can stick with test-driven development and go ahead and write our unit test.

class StringTestStuff: XCTestCase {
    func testStringFormatting() {
        let input = "Is my résumé good enough?"
        let expectedResult = "is-my-r-sum-good-enough-"

        var stringFormatter = StringFormatter()
        stringFormatter.blacklistCharacters = Set(" -&+$,/:;=?@\"#{}|^~[]`\\*()%.!'".characters)
        stringFormatter.replacementCharacter = "-"
        stringFormatter.caseRule = .ConvertToLower
        stringFormatter.asciiRule = .AsciiOnly

        let formattedString = stringFormatter.stringByApplyingFormatting(toString: input)

        XCTAssertEqual(formattedString, expectedResult)
    }
}

We can run the test and see that's it's failing (confirming that the test works):

Of course, much like my previous answer, this test is far from complete.  All this test is doing, though, is simply testing the one known acceptance criteria you've provided.  Realistically, we'd want quite a few tests for this, and we'd want to test several of the different aspects of the StringFormatter individually (applying just the case rule, applying just the ASCII rule, etc), and several variations of different combinations.  We'd also probably want to see some performance tests for the StringFormatter too.  There are going to be multiple ways to accomplish this, and we want to make sure that when we refactor, we've not only not broken things, but we've also not made things run super slow.

But let's not kid ourselves, it would be impossible to ever test this code nearly as completely as the simply isAscii test manages.  We will simply have to pick a set of smart tests and be prepared to add tests as edge cases appear.

So with all this in mind, let's work on making this test pass.  (I know your code would pass the test, but let's see if we can do it without import Foundation).

In order to keep in line with the single responsibility principle, we need to add a couple of helper functions.

First, this function will apply our caseRule property:

private func stringWithCaseConversionApplied(string: String) -> String {
    switch caseRule {
    case .ConvertToLower: return string.lowercaseString
    case .ConvertToUpper: return string.uppercaseString
    default: return string
    }
}

And if our caseRule property is set to an "only" value rather than a "convert" value, this method will determine if any individual character would pass that check:

private func caseFilter(character: Character) -> Bool {
    if caseRule == .LowercaseOnly && String(character).uppercaseString == String(character) {
        return false
    }

    if caseRule == .UppercaseOnly && String(character).lowercaseString == String(character) {
        return false
    }

    return true
}

But we've implemented another method to apply all of the filtering rules:

private func ruleFilter(character: Character) -> Bool {
    if blacklistCharacters.contains(character) {
        return false
    }

    if asciiRule == .AsciiOnly && !character.isAscii {
        return false
    }

    return caseFilter(character)
}

So, now we have all the pieces in place to check individual characters.  It's time to break out some of the Swift standard library functions that will allow us to avoid importing Foundation.

We're back to working in stringByApplyingFormatting(), the method around which we've written our unit test.

We're going to first apply the case conversion rule by calling the method we wrote to do that, then we're going to filter out any illegal characters based on our ruleFilter method.  This will break our string down into an array of character sequences which can be joined back together with our replacementCharacter.  This will eliminate the trouble of repeatedly checking for occurrences of doubles of the replacement character.

The last step looks a little weird.  We need to check if the first or last character was an illegal character, and if so, stick the replacement character at the beginning or end (or both) because the join method will only place it between our sub sequences.  It won't keep track of whether or not it eliminated the first or last characters.

So, the logic within our stringByApplyingFormatting() method now looks like this:

func stringByApplyingFormatting(toString string: String) -> String {
    // Apply case conversion rule
    let caseValue = stringWithCaseConversionApplied(string)

    // Apply the rule filter to each character
    // This splits the string into an array of character subsequences
    let splitCharacters = caseValue.characters.split { !ruleFilter($0) }

    // Map the array of character subsequences to an array of strings that can be joined
    let splitString = splitCharacters.map(String.init)

    // Join the array of strings with the specified replacement character
    let joinedString = splitString.joinWithSeparator(replacementCharacter)

    // Check if the original string started or ended with an illegal character
    // If so, prepend/append the replacement character
    let finalString =
        (!ruleFilter(caseValue.characters.first ?? Character("")) ? replacementCharacter : "") +
        joinedString +
        (!ruleFilter(caseValue.characters.last ?? Character("")) ? replacementCharacter : "")

    return finalString
}

And with this logic, if we re-run our unit test, it's passing now.  And we also no longer need to import Foundation into our code base, so this code runs perfectly well on Linux too.

From here, the next step is adding a pretty significant suite of tests around all of the individual rules and all of the possible combinations of rules, with all sorts of test strings.

The full StringFormatter struct:

struct StringFormatter {
    enum CaseRule {
        case None, UppercaseOnly, ConvertToUpper, LowercaseOnly, ConvertToLower
    }

    enum AsciiRule {
        case None, AsciiOnly
    }

    var blacklistCharacters = Set<Character>()
    var replacementCharacter = ""
    var caseRule = CaseRule.None
    var asciiRule = AsciiRule.None

    func stringByApplyingFormatting(toString string: String) -> String {
        let caseValue = stringWithCaseConversionApplied(string)

        let splitCharacters = caseValue.characters.split { !ruleFilter($0) }

        let splitString = splitCharacters.map(String.init)

        let joinedString = splitString.joinWithSeparator(replacementCharacter)

        let finalString = (!ruleFilter(caseValue.characters.first ?? Character("")) ? replacementCharacter : "") + joinedString + (!ruleFilter(caseValue.characters.last ?? Character("")) ? replacementCharacter : "")

        return finalString
    }

    private func stringWithCaseConversionApplied(string: String) -> String {
        switch caseRule {
        case .ConvertToLower: return string.lowercaseString
        case .ConvertToUpper: return string.uppercaseString
        default: return string
        }
    }

    private func ruleFilter(character: Character) -> Bool {
        if blacklistCharacters.contains(character) {
            return false
        }

        if asciiRule == .AsciiOnly && !character.isAscii {
            return false
        }

        return caseFilter(character)
    }

    private func caseFilter(character: Character) -> Bool {
        if caseRule == .LowercaseOnly && String(character).uppercaseString == String(character) {
            return false
        }

        if caseRule == .UppercaseOnly && String(character).lowercaseString == String(character) {
            return false
        }

        return true
    }
}

The unit test is the same as posted above:

class StringTestStuff: XCTestCase {
    func testStringFormatting() {
        let input = "Is my résumé good enough?"
        let expectedResult = "is-my-r-sum-good-enough-"

        var stringFormatter = StringFormatter()
        stringFormatter.blacklistCharacters = Set(" -&+$,/:;=?@\"#{}|^~[]`\\*()%.!'".characters)
        stringFormatter.replacementCharacter = "-"
        stringFormatter.caseRule = .ConvertToLower
        stringFormatter.asciiRule = .AsciiOnly

        let formattedString = stringFormatter.stringByApplyingFormatting(toString: input)

        XCTAssertEqual(formattedString, expectedResult)
    }
}

As an addendum, one thing that might be neat to add to the StringFormatter struct would be a means for mapping certain characters to be replaced with specified characters.  So, for example, you're not going to allow ASCII, but you could potentially replace è with a "regular" e.

So we might add a property to the struct that looks like this:

var replacementMap = [Character: Character]()

So it's a dictionary where the key is the character we want to remove and the value is the character we want to replace it with.

stringFormatter.replacementMap = [
    "è" : "e",
    "é" : "e",
    "ê" : "e",
    "ë" : "e",
    "ē" : "e",
    "ė" : "e",
    "ę" : "e"
]

Of course, that particular map just covers the characters that might map to "e", and we'd have to do this before we applied the ASCII rule.  But implementation would look something like this:

private func stringByApplyingReplacementMap(string: String) -> String {
    return String(string.characters.map { return self.replacementMap[$0] ?? $0 })
}

For more reading on testing, check out Unit Tests Don't Prevent Bugs.


Checking if CPU supports rdrand My goal with this bit of code is to check if my processor supports rdrand and, if not, execute some other random number generating function. To check if rdrand is supported, the 30th bit in the ecx register should be set.

I guess my one of my dilemmas is whether or not I should explicitly check inside level 1 but I think eax implicitly sets the level there. Additionally, I wonder if I should set initialize the values of the registers to 0. I'd also like to know of any 'gotchas' and possible improvements.

#include <stdio.h>
#include <string.h>

#define bit_RDRND   (1 << 30)

int main(int argc, char **argv)
{

    unsigned int eax;
    unsigned int ebx;
    unsigned int ecx;
    unsigned int edx;

    eax = 0x01;

    __asm__ __volatile__(
                         "cpuid;"
                         : "=a"(eax), "=b"(ebx), "=c"(ecx), "=d"(edx)
                         : "a"(eax)
                         );

    printf("The value of the ecx register is %08x.\n", ecx);

    if(ecx & bit_RDRND){
        //use rdrand
        printf("use rdrand\n");
    }
    else{
        //use mt19937
        printf("use mt19937");
    }

    return 0;
}

Let's look at the inline assembly code that you have:

You were correct to use the __volatile__ specifier here. That's a common mistake when writing inline asm, so that's a good start. The CPUID instruction has very important side-effects, and you don't want the compiler deciding to elide it in the name of "optimization".
(Note: I may be wrong here, and __volatile__ may not actually be necessary for CPUID. The official documentation, as I read it, is rather unclear, and I confess to not fully understanding the nuances. See also discussion in comments below.)
You don't need a semicolon after the cpuid instruction in the inline assembly block. For only a single instruction, no terminator is needed. If you have multiple instructions, standard practice is simply to separate them with \n\t so that they will be nicely formatted in the compiler's intermediate assembly output. A semicolon won't affect the correctness of the code, of course, but they might confuse a human reader of the code who isn't expecting to see one there. They also just clutter up the compiler's intermediate asm output, should you ever need to analyze it.
This is purely stylistic, but I like to align the colons in the inline asm syntax in columns with the opening and closing parentheses. The way you have it now is perfectly readable, though. This starts to matter more when you have multiple instructions in a single asm block.
You also expressed concern in the question about whether you should initialize the values of the registers to 0. In fact, you seem to be slightly confused about exactly how to call CPUID. The authoritative reference is always the Intel manuals, but transcriptions have been made and placed online by multiple folks. Searching Google for the instruction name + x86 makes them very easy to find. Here is the documentation for CPUID. It tells you exactly how it needs to be called.

The summary is, EAX needs to be set to the "function"/"level", which indicates the type of information to retrieve. In your case, to get the feature bits, EAX should contain 1. The other registers do not need to be pre-initialized; they will be clobbered and filled by the CPU during the execution of CPUID. The code you have is therefore correct as-written; there are no hidden "gotchas" to be aware of (at least, as long as you are using CPUID as shown here; it is a very "overloaded" instruction!).
Finally, I know this is probably just demo code (are you expected to post real code here?), but you really should wrap ugliness like this up in a function. Not only because it looks cleaner, but it also helps to consolidate the scope when the need arises to maintain it. Here's how I'd write it (using my preferred naming conventions; use whatever matches your project):

void InvokeCPUID(unsigned int  function,
                 unsigned int  subfunction,
                 unsigned int* pEAX,
                 unsigned int* pEBX,
                 unsigned int* pECX,
                 unsigned int* pEDX)
{
    assert(pEAX != NULL);
    assert(pEBX != NULL);
    assert(pECX != NULL);
    assert(pEDX != NULL);

    __asm__ __volatile__("cpuid"
                        : "=a" (*pEAX),
                          "=b" (*pEBX),
                          "=c" (*pECX),
                          "=d" (*pEDX)
                        :  "a" (function),
                           "c" (subfunction)
                        );
}

_Bool SupportsRDRAND()
{
    const unsigned int flag_RDRAND = (1 << 30);

    unsigned int eax, ebx, ecx, edx;
    InvokeCPUID(1, 0, &eax, &ebx, &ecx, &edx);

    return ((ecx & flag_RDRAND) == flag_RDRAND);
}

The object code emitted by GCC and Clang for SupportsRDRAND (with the call to CPUID inlined) is beautifully simple:

SupportsRDRAND:
    push    ebx        ; preserve caller-save EBX (required by calling convention)
    mov     eax, 1     ; set EAX to request function 1
    xor     ecx, ecx   ; set ECX to request subfunction 0
    cpuid
    shr     ecx, 30    ; the result we want is in ECX...
    and     ecx, 1     ; ...test for the flag of interest...
    mov     eax, ecx   ; ...and put the result in EAX so it can be returned
    pop     ebx        ; restore previously-saved value of EBX
    ret

That said…it is my recommendation that you not use inline assembly at all! I know its raw power is quite seductive, and sometimes you truly have no choice, but you really should try to avoid inline assembly if at all possible. Architectural portability obviously isn't a compelling argument here, but the fact that inline assembly disrupts the optimizer is still important, and even more important is that it vastly increases the development time and maintenance costs. You really do need to be an expert to get the inline assembly code right. And even if you are such an expert (and we still get it wrong regularly), the maintenance programmer who comes along later is invariably not.

Intrinsics are virtually always a better choice, unless you can really demonstrably outsmart the compiler—and even then, only if the code is a real performance bottleneck to justify the maintenance cost. In this case, it is absolutely not necessary. There are intrinsics/built-ins for the CPUID instruction, so you don't need to resort to inline assembly.

Based on the inline assembly syntax used here, you're obviously targeting a Gnu-style compiler. GCC and Clang both support the __get_cpuid built-in, but you'll need to include the <cpuid.h> header to use it. (If you were using MSVC, it has an equivalent __cpuid built-in, in <intrin.h>, that you could use.) Here's how I'd write the code:

#include <cpuid.h>   // for __get_cpuid intrinsic

_Bool SupportsRDRAND()
{
    const unsigned int flag_RDRAND = (1 << 30);

    unsigned int eax, ebx, ecx, edx;
    __get_cpuid(1, &eax, &ebx, &ecx, &edx);

    return ((ecx & flag_RDRAND) == flag_RDRAND);
}

Clang does an outstanding job with this code, producing object code virtually identical to that shown above. On the other hand, seemingly intent on making a liar out of me regarding the virtues of intrinsics, GCC makes an absolute mess of it. The reason for the "mess" is that it is calling CPUID twice: the first time with EAX set to 0 in order to obtain the highest function/level that the CPU supports, and then the second time with EAX set to the requested function/level. It appears to be doing this out of an abundance of caution, and perhaps based on a literal reading of the Intel documentation, which does recommend this. However, as far as I'm concerned, it is completely unnecessary. All CPUs that support the CPUID instruction support at least function/level 1, so the compiler should be able to detect that this is what you've requested and elide the first verificatory call to CPUID. GCC is clearly not making any attempt whatsoever to optimize the translation of the __get_cpuid intrinsic, since even calling it with the function/level set to 0 results in two invocations of CPUID, this time clearly redundant. While this isn't a bug per se, it is a pretty significant missed optimization opportunity.

The good news, though, is that a call to CPUID is never on the critical path for an application, so even the slowest calling code in the world cannot be a bottleneck. This is probably the counter-argument to be made for GCC's failure to optimize __get_cpuid, and means it is not worth losing sleep over. If for some reason you did find yourself losing sleep over this, you could always resort to using inline assembly, as discussed above. Or, you could use the internal __cpuid macro (which is used internally by the __get_cpuid intrinsic):

#include <cpuid.h>     // for __cpuid

_Bool SupportsRDRAND()
{
    const unsigned int flag_RDRAND = (1 << 30);

    unsigned int eax, ebx, ecx, edx;
    __cpuid(1, eax, ebx, ecx, edx);

    return ((ecx & flag_RDRAND) == flag_RDRAND);
}

This brings the object code emitted by GCC into line with my expectations, like that shown above. (Which makes sense, considering the definition of the macro is essentially identical to our inline asm above!) Although I can't find evidence from the Gnu folks that this is documented and intended for public consumption, it does work on GCC, Clang, and ICC. (This is also the name of the corresponding intrinsic for MSVC, although it is called a bit differently.)

Other Notes:

The above discussion of code quality reminds me of an important point that needs to be pointed out explicitly: when using CPUID, you should call it at application initialization to obtain the desired information, and then immediately cache those results away in a global variable. Whenever you want to make dynamic-dispatching decisions during the course of your application's execution, check the global flag—don't re-call CPUID each time! There is no need to pay the speed penalty (even with the most optimal calling code generation, CPUID itself is an extremely slow instruction), because the CPU is not going to change during your application's lifetime.
Notice that I have written my bit-testing code slightly differently than yours. You are just ANDing the mask with the bits, and then testing if the result is non-zero:

if (ecx & flag_RDRAND) { ... }

By contrast, I prefer to explicitly test that the result is equal to the mask:

((ecx & flag_RDRAND) == flag_RDRAND) { ... }

In the majority of cases, the results will be identical, but I prefer this for readability and generality. In the case that you have a multi-bit mask, comparing against zero may lead to an incorrect result, as it will evaluate to non-zero if either of the flags are set. Also, in the event that you ever have a mask of 0 (which you shouldn't, of course), writing the comparison the way I've done will ensure that you get the expected result.

The way I look at it is, writing code is difficult enough already, so the fewer things you have to remember and think about, the better. If you can write bit-testing code that is always correct without ever having to think about it, then I believe that's what you should do.
If possible, prefer not using macros to define constants! I realize there are cases in C where you actually do need to use a #define (or possibly an enum), and a static const variable simply won't work since it's not a true constant (e.g., when declaring the size of a memory array in a structure), but I believe you should prefer const objects whenever possible. These are more debugger-friendly, avoid namespace pollution, respect scoping rules, and are type-safe. Any decent optimizing compiler should fold constants, so the resulting binary will be equivalent. If you're on an embedded system, the rules may be different, but this code is clearly x86-specific!
Dair already noted your use of printf as opposed to puts. I guess he's probably right; I can't argue with the statement that simpler is usually better than more complicated. But this is obviously just demo code, not a demonstration of what you're actually going to be doing with these results. Furthermore, which function you use for console output is just not something that matters in real-world code. Both functions are so slow from an optimization perspective that the potentially-minor speed improvement of puts over printf is unimportant. The bottleneck there is not the speed of the output, but the speed at which a human operator can perceive the stimuli. Besides, a sufficiently smart compiler may optimize printf into puts when possible.

Next Steps:

Finally, harking back to the discussion at the top of why you should always prefer intrinsics to inline assembly, when you get ready to invoke RDRAND on the processors that you determine have support for it, make sure that you use an intrinsic to do so.

The following are the intrinsics for RDRAND, parameterized on the size (in bits) of the random integer to be generated:

int _rdrand16_step(unsigned short*)
int _rdrand32_step(unsigned int*)
int _rdrand64_step(unsigned long long*)

These write the generated random value into to the specified pointer, and return an int indicating success (1) or failure (0). To use them, you must include the <x86intrin.h> header (or, specifically, <immintrin.h>).

Usage is drop-dead simple. If you wanted to retain the ability to check for success, you could do something like:

#include <x86intrin.h>   // for _rdrand32_step intrinsic

_Bool GetRandomViaRDRAND(unsigned int* pResult)
{
    // This function should *only* be called if the underlying processor supports
    // the RDRAND instruction. This is the caller's responsibility to verify,
    // e.g., using the CPUID instruction. For performance reasons,
    // we don't want to verify this each time a random number is desired,
    // but to ensure correctness, we will validate it in debugging builds.
    assert(SupportsRDRAND());

    return (_rdrand32_step(pResult) != 0);
}

Or, if you want to play it dangerously and ignore the hardware's success flag, you could do the following:

unsigned int GetRandomViaRDRAND()
{
    // This function should *only* be called if the underlying processor supports
    // the RDRAND instruction. This is the caller's responsibility to verify,
    // e.g., using the CPUID instruction. For performance reasons,
    // we don't want to verify this each time a random number is desired,
    // but to ensure correctness, we will validate it in debugging builds.
    assert(SupportsRDRAND());

    unsigned int value;
    int result;
    result = _rdrand32_step(&value);
    assert(result != 0);
    return value;
}

This not only simplifies the interface by which the function is called, but also results in slightly more efficient object code. Because you're using an intrinsic, the compiler can tell whether or not you're actually using its return value. If you do, it has to store the carry flag (CF is set by the CPU after executing RDRAND) in such a way that you can test it (compilers use a conditional move instruction for this). If you don't use the return value, it can elide this code altogether, executing only an RDRAND instruction.

If you did need to fall back to inline assembly for RDRAND, hopefully you know how to do it correctly now. Notice that we need to use the cc clobber, since the instruction clobbers the flags:

unsigned int GetRandomViaRDRAND()
{
    unsigned int value;
    __asm__("rdrand  %[value]"
           : [value] "=r" (value)
           :       /* no inputs */
           : "cc"  /* clobbers flags (condition codes) */
           );
    return value;
}


Object-oriented implementation on Expense Tracking App I am creating a Expense Tracking Application in C# and right now I am trying to build its building blocks in a console app. I Would like to know your feedback if it is in a Object Oriented Design/Structure.

namespace ExpenseTrackerOOP
{
    class ExpenseTracker
    {
        static void Main(string[] args)
        {
            Account expense = new Expenses();
            expense.Initialize();

            Account earnings = new Earnings();
            earnings.Initialize();

        }
    }

    public enum ExpenseType
    {
        Food,
        Transportation,
        Utitlities
    }
    public enum AccountTypes
    {
        Expenses,
        Earnings
    }
    public class AccountType
    {
        public AccountTypes Type;
    }

    public abstract class Account
    {
        public decimal Value { get; set; }

        public AccountType Type;

        public abstract void Initialize();
    }

    public class Expenses : Account
    {
        public List<Expenses> Expenses_list { get; set; }

        public override void Initialize()
        {
            Expenses_list = new List<Expenses>(){
                new Expenses() { Value = 10, Type = (new AccountType{Type = AccountTypes.Expenses}) },
                new Expenses() { Value = 20, Type = (new AccountType{Type = AccountTypes.Expenses}) },
                new Expenses() { Value = 30, Type = (new AccountType{Type = AccountTypes.Expenses}) }
            };
        }
    }

    public class Earnings : Account
    {
        public List<Earnings> Earnings_list { get; set; }

        public override void Initialize()
        {
            Earnings_list = new List<Earnings>(){
                new Earnings() { Value = 10, Type = (new AccountType{Type = AccountTypes.Earnings}) },
                new Earnings() { Value = 20, Type = (new AccountType{Type = AccountTypes.Earnings}) },
                new Earnings() { Value = 30, Type = (new AccountType{Type = AccountTypes.Earnings}) }
            };
        }
    }
}

There are several aspects about your code worth reviewing: OO-principles, code conventions, functional design. Since you specifically requested to review OO-design and your question lacks context to review functional design, I'll focus on the former.

Inheritance
Expenses and Earnings both derive from Account to provide their specific implementation of Initialze. Because of a lack of context, I can't make a call about this design. The implementation of both classes does seem off. I don't think you want each instance of these classes to create and store the same fixed list of presets.

Polymorphism
This is proper use of polymorphism. The declaring type Account is the closest fit to enable the use case Initialize. The instance type Expenses provides the specific implementation of that method.

Account expense = new Expenses();
expense.Initialize();

Composition
You have been a bit too diligent in the composition of some of your classes. AccountType is a redundant class, which acts only as a wrapper for AccountTypes Type. Get rid of it and use AccountTypes directly in Account.

public class AccountType
{
     public AccountTypes Type;
}

Encapsulation
Account is an abstract class that does not provide an explicit constructor. A public default constructor is implied. It is impossible for derived classes to reduce the scope of the access modifier, hence it's a breach in encapsulation. Consider providing a protected constructor for abstract classes.
public abstract class Account
{
    protected Account() {}
}

All your properties and fields are mutable. This might be as desiged, but at least think about whether this is a requirement or just lazy design.

public decimal Value { get; set; }


Improving my user class I am still uncomfortable and new with working with classes.

I have made a User class that will return stuff about the user.

<?

class User {
    public function age($id) {
        $birth = new DateTime("$id");
        $now = new DateTime();
        $age = $now->diff($birth)->format("%y");
        return $age;
    }
    public function avatar($id) {
        $connect = Database::factory();

        $id = $id;
        $getQuery = $connect->prepare("SELECT photo_thumb FROM users_profile WHERE uID =:id");
        $getQuery->bindValue(":id", $id);
        $getQuery->execute();
        $show = $getQuery->fetch();
        $photoName = ((empty($show['photo_thumb'])) ? '' : $show['photo_thumb']);
        return $photoName;
    }
    public function fullname($id, $full = 1) {
        $connect = Database::factory();
        $query = $connect->prepare("SELECT firstname, lastname FROM users WHERE id =:id");
        $query->bindValue(":id", $id);
        $query->execute();
        $row = $query->fetch();

        $full_name = $row["firstname"]." ".$row["lastname"];
        return $full_name;
    }
    public function firstname($id) {
        $connect = Database::factory();
        $query = $connect->prepare("SELECT firstname, lastname FROM users WHERE id = '$id'");
        $query->execute();
        $row = $query->fetch();
        $firstname = $row["firstname"];
        return $firstname;
    }
    public function sex($id) {
        $connect = Database::factory();

        $query = $connect->prepare("SELECT sex FROM users WHERE id=:id");
        $query->bindValue(":id", $id);
        $query->execute();
        $row = $query->fetch();
        return $row["sex"];
    }
}

If you look through it fast, you will notice how everything is like the same, apart from its grabbing another column name and sometimes from another table.

Otherwise it just execute, fetches and returns.

I wonder if I can improve this code any way, so it looks cleaner and not so copy+paste like now..

In short : its an extremely bad piece of code.

Here is a list or "whys":

you are mixing data access with logic ( sql queries should be handled by different object )
you are using global state for DB access
there is some mystical parameter $full in full_name() method
all the methods are getters
the SQL queries happen each time you use a getter
parameter $id means nothing ( is it a string , a number , some other object ? )

Basically what you have there are five standalone procedures wrapped up in an a class just for show.


Finding the value 'n' in an array that has 'n' or more larger values Given an unsorted array of integers, you need to return maximum possible \$n\$
  such that the array consists at least \$n\$ values greater than or equals
  to \$n\$. Array can contain duplicate values.

  Sample 1

input : [1, 2, 3, 4]
output: 2

  Sample 2

input : [900, 2, 901, 3, 1000]
output: 3

What do you folks think of my code to this problem?

      public static Integer findN(Integer[] values) {
        Comparator comparator = new Comparator<Integer>() {
          @Override
            public int compare(Integer o1, Integer o2) {
              return o2.compareTo(o1);
            }
        };
        Arrays.sort(values, comparator);
        for (int i = 0 ; i < values.length; i++) {
            if (i >= values[i]) {
                return values[i];
            }
        }
        return Integer.MIN_VALUE;
      }

Right now, after you sort the array, you're doing a linear search for the correct value. I believe you should be able to write that part of the task using a binary search instead of a linear search.

The sorting has a minimum complexity of \$O(N log N)\$, so reducing the search from \$O(N)\$ to \$O(log N)\$ won't affect the overall computational complexity, but for any more than a small number of items, it should still reduce the overall time.


Mutative Heap's algorithm (permutations generator) in Clojure For a side project I'm working on, I need to be able to generate permutations. Instead of relying on an existing library like math.combinatorics, I decided to write my own implementation, and arbitrarily decided on Heap's Algorithm.

Example:

(permutations [1 2 3])
=> [[1 2 3] [2 1 3] [3 1 2] [1 3 2] [2 3 1] [3 2 1]]

I realized pretty quickly though that this algorithm relies heavily on the mutation of an array, and ended up giving up trying to adapt it to proper FP style.

I went to the dark side and made an implementation that relies on two atoms to function.

Now, the mutable array part seems pretty necessary to the algorithm, but I ended up needing a second atom to accumulate the permutations. I tried adapting the doseq part to a reduction or just using loop, but the recursive calls are such that it's very difficult to see how I'd pass data along back up the stack.

Mainly, I'd like advice on how I can get rid of the need for result-atom, but I'd welcome any critique here.

I'm fully aware that this is going strongly against the FP grain here, but I seemed forced into this corner trying to implement this algorithm. I'd also, if possible, like to see a fully FP implementation of this, but I'd wager that that would be quite difficult.

I'm also aware that Heap's algorithm may not be suited for FP, but that's ok. After like half an hour of fiddling, I became more focused on producing a working implementation of this algorithm, and less whether or not this was the right algorithm to be using in the first place.

(ns wof-guesser.logic.permutations)

(defn- swap-v [v i1 i2]
  (let [x (get v i1)]
    (-> v
        (assoc i1 (get v i2))
        (assoc i2 x))))

(defn- mutative-permutate [coll-atom result-atom]
  ((fn rec [n]
     (when (> n 1)
       (let [n-even? (zero? (rem n 2))
             swap-pos #(if n-even? % 0)]

         (doseq [i (range (dec n))]
           (rec (dec n))
           (swap! coll-atom swap-v (swap-pos i) (dec n))
           (swap! result-atom conj @coll-atom))

         (rec (dec n)))))

   (count @coll-atom)))

(defn permutations [coll]
  (let [v-coll (vec coll)
        col-a  (atom v-coll)
        res-a  (atom [v-coll])]
    (mutative-permutate col-a res-a)
    @res-a))

I'd also, if possible, like to see a fully FP implementation of this,
  but I'd wager that that would be quite difficult.

Here is a functional implementation of Heap's algorithm in Clojure, based on page 12 of Robert Sedgewick's lecture (Don't look at the next page - the code is dubious).

The idea is to generate the sequence of swaps as pairs of indices. That's what the swaps function does. For an argument n, it generates n! - 1 of them:

(defn swaps [n]
  (if (= n 1)
    ()
    (let [base (swaps (dec n))
          extras (if (odd? n) (repeat (dec n) 0) (range (dec n)))]
      (concat
        base
        (mapcat (fn [x] (cons [x (dec n)] base)) extras)))))

It interleaves the extras between n copies of the base, the sequence of swaps of n-1 elements.

For example,

=> (swaps 3)
([0 1] [0 2] [0 1] [0 2] [0 1])

The perms function ...

(defn perms [v]
  (reductions
    (fn [a [i j]] (assoc a i (a j) j (a i)))
    v
    (swaps (count v))))

... simply applies these swaps successively to its vector argument.

For example,

=> (perms ['A 'B 'C])
([A B C] [B A C] [C A B] [A C B] [B C A] [C B A])


Thread synchronization wait/notify I need to run several identical threads. All threads must wait to do their job, until all threads are running (alive).
I use wait/notify to synchronize: when a thread is created it waits until it gets a notify to continue.

It works, but not sure it's the best way. Is it efficient?
I read from Java 5, is it better to use concurrency package. Is it true?

public class MainTest {

    public static void main(String[] args) throws InterruptedException {

        TestTH[] ths = new TestTH[5];

        System.out.println("== Start == ");

        for (int i = 0; i < ths.length; i++) {
            ths[i] = new TestTH("Name_" + i, new Object()); //<- Creates runnable class and lock object that will be used to synchronize
            ths[i].start(); //<- Creates and starts thread
        }

        Thread.sleep(100L); // <- All threads are running

        for (int i = ths.length -1 ; i >= 0; i--) {
            Object lock  = ths[i].getLock();
            synchronized (lock) {
                lock.notify();  //<- Thread i, can continue;
            }
            ths[i].getThread().join(); // Wait until thread i, finishes
        }

        System.out.println("== End == ");
    }

}

class TestTH implements Runnable {
    private String name;
    private Object lock;
    private Thread t;

    public TestTH(String name_, Object lock_) {
        this.name = name_;
        this.lock = lock_;
        System.out.println("Create thread " + this.name);
    }

    public Object getLock() {
        return this.lock;
    }

    public Thread getThread () {
        return t;
    }

    public void start() {
        if (t == null) {
            t = new Thread(this, name);
            t.start();
        }
    }

    @Override
    public void run() {
        System.out.println("Init thread  " + this.name);

        synchronized (lock) {
            try {
                lock.wait(); // <- Once started, it waits current until notify.
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        }

        // doStuff(); // <- do stuff when lock is released

        System.out.println("End thread " + this.name);
    }

}

Any advice, will be very welcome.

Concurrent programming adds a lot of complexity only by the fact that it is concurrent - this alone is reason enough to use concurrency API. One of the rules of good programming is to not reinvent a wheel - which means

always use available libraries instead of coding yourself unless you have a very good reason to do it

This applies above all to JDK, in your case to the java.util.concurrent package.

Here is a very simple solution, which is so simple only because of concurrent package in JDK - it uses lambda expression so you'll need Java 8, but it's easy to replace is with a Runnable:

import java.util.Random;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.CyclicBarrier;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;

public class ThreadStartTest {

  public static void main(String[] args) {
    int count = 5;

    CountDownLatch latch = new CountDownLatch(count);
    CyclicBarrier barrier = new CyclicBarrier(count);
    ExecutorService pool = Executors.newFixedThreadPool(count);

    Random random = new Random();
    for (int i = 0; i < count; i++) {
      pool.execute(() -> {
        try {
          System.out.println("Thread starts");
          // startup
          Thread.sleep(random.nextInt(100));
          // wait for other threads
          System.out.println("Thread waits");
          barrier.await();
          // process whaterver should be processed
          System.out.println("Thread works");
          Thread.sleep(random.nextInt(1000));
          System.out.println("Thread finished");
        } catch (Exception e) {
          System.err.println("Worker thread inrerrupted");
        } finally {
          latch.countDown();
        }
      });
    }

    try {
      // wait for the threads to be done
      latch.await();
      System.out.println("== End == ");
    } catch (InterruptedException e) {
      System.err.println("Starting inrerrupted");
    }
    pool.shutdown();
  }

}

It does all you need without the need to cope with complex lock/notify sequences with possible deadlocks - just using what the language offers.

If you increase the number of threads you'll notice how the threads start to wait before all even got started, yet all wait for other before starting processing.

Explanation

First let me stress that the concurrent package has very powerful asynchronous concepts that make concurrent programming almost easy :-), so I would suggest trying to get familiar with it and refrain from complex and hard to debug state machines with lock/notify.

Now here we have some constructs from the java.util.concurrent package playing together:

ExecutorService
CyclicBarrier
CountDownLatch

Simple speaking the ExecutorService manages a pool of threads and distributes work between them – it is not guaranteed that each work unit gets a new thread though. Anyway it frees you from creating threads on your own, additionaly you can not only execute Runnable without a return value but also Callable instances, where you can get a return value using Futures. This is powerful, istn't it - submit code to be executed asynchronously and retrieve the result at some time in the future - all that in a very short and simple and expressive code.

In this case the ExecutorService gets created with Executors.newFixedThreadPool(count), so we have exactly the same number of threads as work units, which is important - more about it later.

From Javadoc:

  CyclicBarrier is a synchronization aid that allows a set of threads to
  all wait for each other to reach a common barrier point.
  CyclicBarriers are useful in programs involving a fixed sized party of
  threads that must occasionally wait for each other.

This is exactly the case here – you wish that all your threads wait for each other before continuing. The parameter to the constructor defines how many threads should wait – in this case exactly the same value as the number of started threads, so each of the threads waits for the count threads get to to that point by calling barrier.await(). After calling the barrier.await() count times the barrier and thus all the threads are released and can continue.

The CyclicBarrier can be reused, this is why it is called “Cyclic” – just call reset() and you can repeat the whole operation.

Again from the JavaDoc:

  CountDownLatch is a synchronization aid that allows one or more
  threads to wait until a set of operations being performed in other
  threads completes.

In this case the main thread waits in latch.await until all the threads finish the work and let the latch’s counter get down to zero by calling latch.countDown().

Actually we could make a version without the CountDownLatch – which is then even simpler and uses the ExecutorService awaitTermination method – here the ExecutorService waits until all its threads are finished, so that they do not need the latch. The one drawback of this strategy is that we have to actively cope with timeouts, since the awaitTermination method requires a timeout parameter:

Version without CountDownLatch

public static void main(String[] args) {
  int count = 50;

  CyclicBarrier barrier = new CyclicBarrier(count);
  ExecutorService pool = Executors.newFixedThreadPool(count);

  Random random = new Random();
  for (int i = 0; i < count; i++) {
    pool.execute(() -> {
      try {
        System.out.println("Thread starts");
        // startup
        Thread.sleep(random.nextInt(10));
        // wait for other threads
        System.out.println("Thread waits");
        barrier.await();
        // process whaterver should be processed
        System.out.println("Thread works");
        Thread.sleep(random.nextInt(1000));
        System.out.println("Thread finished");
      } catch (Exception e) {
        System.err.println("Worker thread inrerrupted");
      }
    });
  }

  pool.shutdown();
  try {
    // wait 1 minute for the threads to be done
    pool.awaitTermination(1, TimeUnit.MINUTES);
    System.out.println("== End == ");
  } catch (InterruptedException e) {
    System.err.println("Starting inrerrupted");
  }
}

Now this solution has its own disadvantage: if the thread pool's size is smaller than the number of threads it will dead lock in barrier.await(), because all running threads are waiting for more to come yet no more threads can get executed since the pool has no more available threads. You can try it yourself:

ExecutorService pool = Executors.newFixedThreadPool(count - 1);

And it hangs!

Now I have a 3rd version for you, which doesn’t deadlock – to remove the factor limiting the number of threads I’ll give on ExecutorService up and create Threads manually, in this case I have to revert back to CountDownLatch to be able to wait for the threads to finish:

Version without ExecutorService

public static void main(String[] args) {
  int count = 50;

  CountDownLatch latch = new CountDownLatch(count);
  CyclicBarrier barrier = new CyclicBarrier(count);

  Random random = new Random();
  for (int i = 0; i < count; i++) {
    new Thread(() -> {
      try {
        System.out.println("Thread starts");
        // startup
        Thread.sleep(random.nextInt(10));
        // wait for other threads
        System.out.println("Thread waits");
        barrier.await();
        // process whaterver should be processed
        System.out.println("Thread works");
        Thread.sleep(random.nextInt(1000));
        System.out.println("Thread finished");
      } catch (Exception e) {
        System.err.println("Worker thread inrerrupted");
      } finally {
        latch.countDown();
      }
    }).start();
  }

  try {
    // wait for the threads to be done
    latch.await();
    System.out.println("== End == ");
  } catch (InterruptedException e) {
    System.err.println("Starting inrerrupted");
  }
}

Even in this version it remains pretty short and simple thanks to lambda in Java 8.


Move one element before another in an associated array I have this task where I have an associated array in PHP and I want to manipulate the order of entries. Basically I want to be able to move entry Y before entry X.

So basically if I have the following array:

['foo1' => 'bar', 'foo2' => 'buzz', 'foo3' => 'bazz']

I want to be able to say: 'put element foo3 before foo2', resulting in the following:

['foo1' => 'bar', 'foo3' => 'bazz', 'foo2' => 'buzz']

I currently have the following code:

/**
 * Change the order of an associated array, moving by key before another
 *
 * @param array $arr
 * @param string $key
 * @param string $before
 * @return array
 */
public function moveKeyBefore(array $arr, string $key, string $before)
{
    $keys = array_keys($arr);
    if (in_array($key, $keys) && in_array($before, $keys)) {
        $newArr = [];
        foreach ($arr as $idx => $value) {
            switch ($idx) {
                case $key :
                    continue;
                case $before:
                    $newArr[$key] = $arr[$key];
                    $newArr[$before] = $value;
                    break;
                default:
                    $newArr[$idx] = $value;
                    break;
            }
        }

        return $newArr;
    }

    return $arr;
}

This code basically works. However, I was wondering if there was an easier, less complex way of doing this.

I wouldn't bother with a foreach loop because it will do more iterating than necessary.  I recommend halving the array at the desired position and squeezing the element-to-be-moved into the gap created.  Consider this much more concise method:

Code: (Demo Link)

function moveKeyBefore($arr, $find, $move) {
    if (!isset($arr[$find], $arr[$move])) {
        return $arr;
    }

    $elem = [$move=>$arr[$move]];  // cache the element to be moved
    $start = array_splice($arr, 0, array_search($find, array_keys($arr)));
    unset($start[$move]);  // only important if $move is in $start
    return $start + $elem + $arr;
}
$arr = ['foo1'=>'bar', 'foo2'=>'buzz', 'foo3'=>'bazz'];
var_export(moveKeyBefore($arr, 'foo3', 'foo1'));

Output:

array (
  'foo2' => 'buzz',
  'foo1' => 'bar',
  'foo3' => 'bazz',
)

Explanation:

First check that both of the nominated keys exist in the array, if not return the input array untouched.  isset() is very fast, certainly faster than in_array().  Furthermore, the !isset() check will require both keys to exist otherwise the early return will be triggered.  This is as fast as this part gets.

Next, if both $find and $move exist in the array, my function will generate three array fragments:

$elem contains the associative array element declared by $move.
$start contains the elements that precede $find.  array_search() returns the offset (numeric key) of the $find value in $arr.
$arr is modified by array_splice() and contains all elements from find to the end.

After the splice, when $move is in the $arr half, the union action bumps the duplicate element out.  When $move is in the $start half, the convenient "bump" is lost, this means that the $move element needs to be removed from its original location.  unset() can be used unconditionally because it doesn't error when the target element is not found.


PHP script that combines data from two CSV files to produce two new files I've spent some time looking at various posts on Stack Overflow regarding PHP CSV file manipulation and have devised a script that combines data from two CSV files to create two new files.

The aim of the task was to combine voucher numbers from one file to the customer data in another, along with the current date. I achieved this by extracting all the necessary data and writing it to a new file.

As some additional security/record keeping I've also combined the voucher data with the customer ID and current date to another file, for reference if needed.

I'm relatively new to PHP so your feedback would be greatly appreciated so that I might improve my code/logic:

// Open files for reading
$customers = fopen("customerTest1.csv", "r");
$vouchers = fopen("vouchersExtra.csv", "r+");

// Create new files to write data
$customerOutput = fopen("customerUpdated.csv", "w");
$voucherOutput = fopen("vouchersUpdated.csv", "w");

// Get columns from customer data and voucher data
$customerColumns = fgetcsv($customers, 1000);
$voucherColumns = fgetcsv($vouchers, 1000);

// Add extra columns for the output files
array_push($customerColumns, "voucherNumber", "dateApplied");
array_push($voucherColumns, "id", "dateUsed");

// Write the column headers to both customer and voucher output files
if (!$voucherOutput && !$customerOutput) {
    die("$voucherOutput and $customerOutput BROKE");
} else {
    fputcsv($customerOutput, $customerColumns);
    fputcsv($voucherOutput, $voucherColumns);
}

// Open the input
if (!$customers) {
    die("$customers BROKE");
} else {

    while (!feof($customers)) {

        // Extract current customer row of data
        $currentCustomer = fgetcsv($customers, 1000);

        // Only run the code if a current customer row exists
        if ($currentCustomer != false) {

            // Extract current voucher row of data
            $currentVoucher = fgetcsv($vouchers, 1000);

            // Get current customer's ID
            $customerId = $currentCustomer[0];

            // Merge customer and voucher data and push a date string to the end of the array
            $newCustomerRow = array_merge($currentCustomer, $currentVoucher);
            array_push($newCustomerRow, date("l jS \of F Y h:i:s A"));

            // Merge voucher data, customer ID and push a date string to the end of the array
            $newVoucherRow = [];
            array_push($newVoucherRow, $currentVoucher[0], $customerId, date("l jS \of F Y h:i:s A"));

            // Write new customer data to output file
            fputcsv($customerOutput, $newCustomerRow);

            // Write new voucher data to output file
            fputcsv($voucherOutput, $newVoucherRow);

        }

    }

}

There are a few things you should take into account.

You never close your files. At the end, use fclose;
Use try {...} catch(){} finally {} and close the file(s) in the finally
Use functions to enhance readability
Do not use die or exit, this kills the script and is bad practise. See this blog post, very valuable
It might be wise to put "magic strings" in a constants file and including this with require - for example your datetime formats, csv columns and so forth. This makes sure that you do not have to edit the script when any of these would change but rather only change the constants.
Comments rot very quickly, any change would leave you with "you have to update the comments too". Don't use comments, in stead use functions that have self explanatory names. i.e. pushCustomerDateToCsvRow(param1, param2,... )


Increment struct value using reflection The main idea is, I have a struct and I will get a struct field as a string from another function, which I shall increment its value by 1.
Below is the code I used and I think it is memory consuming.
How can I improve it? Is there a better way than the reflection from the start?
Thanks
package main
import (
    "fmt"
    "reflect"
)

func main() {

    type chrinfo struct {

    Size0_50    int
    Size50_100  int
    Size100_1000    int
    Size1000_10000  int
    Size10000   int
    }

    mychr := chrinfo{}
    n := "Size1000_10000"

    y := reflect.ValueOf(&mychr).Elem().FieldByName(n)
    reflect.ValueOf(&mychr).Elem().FieldByName(n).SetInt(int64(y.Interface().(int))+1)

    fmt.Println(mychr, y)
}

OK, after reading you're using a struct because maps use more memory, and that you posted this question here because you're new to go, let's dive in to this a bit more.
Do maps use more memory
The short answer is yes. However, things are a bit more complex than that. In essence, a map is a hash table, so of course, a map of type map[string]int will internally store all the strings, the buckets, hashes, data tables, etc... Whereas a struct is just a block of memory where fields point to an offset within that memory, it's pretty much the most efficient way to group values.
That said, while a map with 5 keys does use more memory than a struct with 5 fields of type int, the difference on modern systems is quite negligible. There are a number of other factors to consider when it comes to choosing the type.
Runtime memory management
Once again, this may seem like a point where maps look like a terrible option, but it's just some background so you understand how maps, WRT memory management, work. Consider this:
foo := map[string]int{
    "one":   1,
    "two":   2,
    "three": 3,
}

What happens here is that the runtime will allocate memory to hold the three strings, and the three values. The strings are passed through a hashing function, all memory required to store this map, the hashes, and its three values is allocated and assigned. Easy. Now compare this to:
bar := map[string]int{}
// some code
bar["one"] = 1
// more core
bar["two"] = 2
// etc...
bar["three"] = 3

In this scenario, the runtime creates an empty map and assigns it to bar. Some memory is allocated for the internal structures the map needs, but each time we assign a new key, the memory required to store the data for this map grows, and the runtime potentially has to reallocate memory, move stuff around, etc... this definitely is less efficient. We can, however, initialise a map with a certain size. I often do this if I know the map I'm populating will eventually hold at least 10, and at most 15 values. I'll just use make and specify the max size:
foobar := make(map[string]int, 3) // guarantees no realloc while adding the first 3 values

Another area where maps put some added strain on the runtime is when it comes to garbage collection. Remember: we need to allocate memory for the value, hash/buckets (map internals), and key, so doing something like:
delete(foo, "two")

affects 3 objects the runtime is having to manage. That's the reality of maps. The go implementation is fairly optimised, but if you run a heavy application that were to rely solely on maps, your pprof data after a while might show that the runtime is spending a lot of time on its GC cycles as a result.
The latter is not really something you should be taken into consideration when comparing maps to structs, however. Structs don't support deleting fields to begin with. I just mentioned this for completeness sake. While it looks like a downside, you could equally argue that having the ability to delete keys from a map is a feature you might want to have. Indeed, sometimes you need to be able to do that.
Restrictions of a Struct
So far, we've established structs are more memory efficient, generally easier on the GC cycles (discounting nested structs, interfaces, embedding, etc...). They do have their limitations, however. The thing discussed earlier about memory management (adding more keys to a map causing reallocation) aren't an issue here. The memory is allocated all at once, and is nicely padded. On the flip side: the memory is allocated all at once. Even if I only were to need one field, a struct is an all or nothing type.
Dynamic access
Maps can use strings, ints, and really any comparable type. A struct has fields. This means the data contained stored in a map is a lot easier to access dynamically:
// given foo from earlier
func get(k string) (int, error) {
    v, ok := foo[k]
    if !ok {
        return 0, fmt.Errorf("key '%s' not found", k)
    }
    return v, nil
}

Whereas using a string to fetch the field of a struct requires a lot more code. The easiest way would be a receiver function on the struct type (method)
// assuming the chrinfo type from your code
func (c chrinfo) get(k string) (int, error) {
    switch k {
    case "Size0_50", "size0_50": // accounting for capitalisation
        return c.Size0_50, nil
    case "Size50_100", "size50_100":
        return c.Size50_100, nil
    case "Size100_1000", "size100_1000":
        return c.Size100_1000, nil
    case "Size1000_10000", "size1000_10000":
        return c.Size1000_10000, nil
    case "Size10000", "size10000":
        return c.Size10000, nil
    }
    return 0, fmt.Errorf("field '%s' doesn't exist", k)
}

This function will need to be updated for each field you add, and although capitalisation is something you may want to take into account, it's cumbersome. Typo's can lead to vague bug reports that cost hours to replicate, only to conclude there was no bug in the code you were looking at, but the person filing the issue didn't bother to check whether or not their input was correct...
Reflection
So obviously, one may conclude that using reflection at least takes away the burden of having to maintain this type of getter function. However, let's circle back to your original reason for not using a map: memory consumption.
Compared to the runtime cost reflection incurs, the memory overhead of a map is laughable. It's akin to hopping on a plane to the arctic to cool a can of coke, instead of using an ice-cube, because using a freezer to freeze ice is wasting electricity, all the while burning literally tons of kerosine to fly your beverage around the globe instead.
Reflection is crazy expensive. It uses a hell of a lot of CPU cycles, and if you look into the source code every time you access something through reflection, you'll allocate the reflect types, like the reflect.Value type, which contains a reflect.rtype. These are just 2 structs you're allocating just to get access to a single field:
type Value struct {
    // typ holds the type of the value represented by a Value.
    typ *rtype

    // Pointer-valued data or, if flagIndir is set, pointer to data.
    // Valid when either flagIndir is set or typ.pointers() is true.
    ptr unsafe.Pointer

    // flag holds metadata about the value.
    // The lowest bits are flag bits:
    //  - flagStickyRO: obtained via unexported not embedded field, so read-only
    //  - flagEmbedRO: obtained via unexported embedded field, so read-only
    //  - flagIndir: val holds a pointer to the data
    //  - flagAddr: v.CanAddr is true (implies flagIndir)
    //  - flagMethod: v is a method value.
    // The next five bits give the Kind of the value.
    // This repeats typ.Kind() except for method values.
    // The remaining 23+ bits give a method number for method values.
    // If flag.kind() != Func, code can assume that flagMethod is unset.
    // If ifaceIndir(typ), code can assume that flagIndir is set.
    flag

    // A method value represents a curried method invocation
    // like r.Read for some receiver r. The typ+val+flag bits describe
    // the receiver r, but the flag's Kind bits say Func (methods are
    // functions), and the top bits of the flag give the method number
    // in r's type's method table.
}

type rtype struct {
    size       uintptr
    ptrdata    uintptr // number of bytes in the type that can contain pointers
    hash       uint32  // hash of type; avoids computation in hash tables
    tflag      tflag   // extra type information flags
    align      uint8   // alignment of variable with this type
    fieldAlign uint8   // alignment of struct field with this type
    kind       uint8   // enumeration for C
    // function for comparing objects of this type
    // (ptr to object A, ptr to object B) -> ==?
    equal     func(unsafe.Pointer, unsafe.Pointer) bool
    gcdata    *byte   // garbage collection data
    str       nameOff // string form
    ptrToThis typeOff // type for pointer to this type, may be zero
}

I'm not going to walk through the entirety of the reflect package, although you may want to take some time to do that yourself, just to see what using reflection actually implies. For now, let me assure you this:
Using reflection uses tons of CPU cycles and memory

So the question you had was: is there a better way? I'd say the answer to that is a resounding yes. A map will do just fine, in fact. The slightly higher memory consumption is nothing compared to the overhead you're creating using reflection.
I touched on tags in my comment, which is something you can look into, but I wouldn't recommend it for this particular use-case (seeing as tags require reflection, too). You could use the combination of a map/struct as a field lookup table, like the one I mentioned. Assuming these calls don't happen concurrently, you can create a map statically (as in: as a field of a type, or a global variable if there is no other option), and do something like this:
var (
    mychr  crinfo          // your struct
    lookup map[string]*int // lookup map
)

func init() {
    lookup = map[string]*int{
        "Size0_50":       &mych.Size0_50,
        "Size50_100":     &mych.Size50_100,
        "Size100_1000":   &mych.Size100_1000,
        "Size1000_10000": &mych.Size1000_10000,
        "Size10000":      &mych.Size10000,
    }
}

Then, to increment the corresponding field, it's quite simple:
func inc(k string) {
    if ptr, ok := lookup[k]; ok {
        *ptr += 1
    }
}

The map contains pointers to each corresponding field, if the field exists, we increment its value. Job done.


Simple cache dictionary I decided to write a simple dictionary to use as a cache:

public class CacheDictionary : Dictionary<string, object>, ICacheDictionary
{
    private readonly Dictionary<string, CancellationTokenSource> _expireTasks = new Dictionary<string, CancellationTokenSource>();
    private int _defaultExpiration;

    public CacheDictionary()
    {
        _defaultExpiration = 30;
    }

    public CacheDictionary(int defaultExpiration)
    {
        _defaultExpiration = defaultExpiration;
    }

    public T GetOrDefault<T>(string key, Func<T> createDefault)
    {
        return GetOrDefault(key, createDefault, TimeSpan.FromSeconds(_defaultExpiration));
    }

    public T Set<T>(string key, Func<T> create)
    {
        return Set(key, create, TimeSpan.FromSeconds(_defaultExpiration));
    }

    public T GetOrDefault<T>(string key, Func<T> createDefault, TimeSpan expireIn)
    {
        return (T)(ContainsKey(key) ? this[key] : Set(key, createDefault, expireIn));
    }

    public T Set<T>(string key, Func<T> create, TimeSpan expireIn)
    {
        if (_expireTasks.ContainsKey(key))
        {
            _expireTasks[key].Cancel();
            _expireTasks.Remove(key);
        }

        var expirationTokenSource = new CancellationTokenSource();
        var expirationToken = expirationTokenSource.Token;

        Task.Delay(expireIn, expirationToken).ContinueWith(_ => Expire(key), expirationToken);

        _expireTasks[key] = expirationTokenSource;

        return (T)(this[key] = create());
    }

    private void Expire(string key)
    {
        if (_expireTasks.ContainsKey(key))
            _expireTasks.Remove(key);

        Remove(key);
    }
}

The functionality is simple: I can add items into the dictionary and set a timer to expire them. I am wondering if I did everything right. Should I also implement IDisposable and remove expiration tasks on Dispose? And if I want it to be thread-safe, should I use ConcurrentBag?

Thread Safety

Yes, there is a possible race condition here if two threads call Set with the same key. But just using a ConcurrentDictionary won't necessarily be enough, since you're effectively managing a transaction across two dictionaries - the main cache and the expiration task dictionary.

There are several ways to avoid this. One is to use lock around the entire Set statement so that concurrent Set requests on the same key will wait until the first call finishes, and the immediately expires the previous item and replaces it.

A second is to avoid having two dictionaries, and not expose IDictionary in the first place, which I think is the best way to go, for one big reason:

You're currently exposing two very different interfaces in your CacheDictionary. Both the standard IDictionary's Add/[] methods for directly setting/fetching a value, and the ICacheDictionary's Get/Set interface, which is similar to ConcurrentDictionary, and deals with value factories, default values, and a whole set of concepts that IDictionary doesn't deal with, regardless of the actual expiration logic. This can be very confusing to your user, who now has two different ways to do the same thing. If I was using this ICacheDictionary, I wouldn't know if calling myDict[key] = value directly was a good idea or not.

What I would do is just implement ICacheDictionary's Get/Set methods, and keep an internal ConcurrentDictionary<string, Tuple<object, TaskCancellationToken>>. This means you only have one data store, which is thread-safe, and it stores both the value and the expiration task for each entry.

Disposability

Generally speaking, there are two good indicators that you should implement IDisposable to clean up your state. The first is that you're using COM objects/Win32 calls/PInvoke statements to create unmanaged resources. The second is that you're holding instances of IDisposable objects that you might want to dispose explicitly when the container is disposed.

In your case, you don't have any unmanaged code. And as of managed objects, you have two types here - the Dictionary, which doesn't require disposal, and the Task objects, which you don't even bother storing, instead relying on the TaskCancellationSource to handle their cancellation with no way to catch or handle them. Since the official .NET recommendation is to not bother with Disposing Task objects, I think you're in the clear here too.

So no, don't bother implementing IDisposable. You can add a Clear method to clear all items, but that's not the same.