Non-sequential template class container C++ I'm pretty new to templates, and admittedly I'm not particularly good at C++, but I've written a little container that does not move allocated objects once they are created, so you can use weak pointers to access elements, and be aware of their existence. I'm sure that much of it can\should be improved, but I lack the experience to know what exactly. Thoughts and critique are welcome.

#include <memory>
#include <vector>
#include <iostream>

class NSC_OOR_ERR : public std::exception
{
    virtual const char* what() const throw()
    {
         return "NSContainer access out of range";
    }
};

class NSC_BA_ERR : public std::exception
{
    virtual const char* what() const throw()
    {
        return "NSContainer out of memory";
    }
};

template<typename A_Type>
class CNSContainer
{
    NSC_OOR_ERR NSC_OOR_m;
    NSC_BA_ERR NSC_BA_m;
    std::vector<std::shared_ptr<A_Type>> pointers_m;
public:
    size_t size()
    {
        return pointers_m.size();
    }

    template <class... Args>
    std::weak_ptr<A_Type> push(Args&&... args)
    {
        try {
            std::shared_ptr<A_Type> ptr(new A_Type(std::forward<Args>(args)...));
        }
        catch (std::bad_alloc& ba) {
            throw NSC_BA_m;
        }
        pointers_m.emplace_back(ptr);
        return ptr;
    }

    void remove(const size_t& key)
    {
        pointers_m.erase(pointers_m.begin() + key);
    }

    std::weak_ptr<A_Type> operator[](const size_t& key)
    {
        try {
            return pointers_m[key];
        }
        catch (const std::out_of_range& oor) {
            throw NSC_OOR_m;
        }
    }
};

int Test()
{
    CNSContainer<int> container;

    //push into container
    container.push(1);
    container.push(2);
    container.push(3);
    container.push(4);

    //get size
    size_t container_size = container.size();

    //iterate
    for (size_t i = 0; i < container_size; ++i)
    {
        std::weak_ptr<int> wptr = container[i];

        if (!wptr.expired())
        {
            auto sptr = wptr.lock();
            int value = *sptr;
            std::cout << "Value is " << value;
        }
    }

    /*take element pointer to do something with it later.
    Maybe in another thread*/
    std::weak_ptr<int> wptr = container[0];

    /*the pointer now points to nothing*/
    container.remove(0);

    /*but we know that, because it's expired*/
    if (!wptr.expired())
    {
        //do stuff
    }

    return 0;
}

Thanks for a well-presented question!  I'll dive straight in:

Your exceptions are a bit weird.  Leaving aside the impenetrable (to me) naming, I think that adding your own exception types derived directly from std::exception just to add your own what() is probably a poor investment of time.  If you go this way, I certainly recommend that you subclass the provided std::bad_alloc and std::out_of_range, so that existing code that catches those also catches your equivalents.

Having member variables of exception type in your container instances is decidedly unconventional.  We generally create exceptions at the point where they are thrown.  This may be important in a debugging environment, if exceptions there are able to carry context information (as happens in Python and Java, for example).  In production code, the exception construction will be very low overhead compared with actually raising and catching the exception, so there's little to be gained in speed, and something to be lost by adding unnecessary members to every instance.

A quick skim through the rest of the code draws me to this line:

        std::shared_ptr<A_Type> ptr(new A_Type(std::forward<Args>(args)...));

Bare new in modern C++ always triggers my interest - here, it is better to use std::make_shared().  This will give improved memory locality to the object and its smart-pointer control block.

This gives my version of your code as follows:

#include <memory>
#include <vector>

template<typename A_Type>
class CNSContainer
{
    std::vector<std::shared_ptr<A_Type>> pointers_m;
public:
    size_t size()
    {
        return pointers_m.size();
    }

    template <class... Args>
    std::weak_ptr<A_Type> push(Args&&... args)
    {
        // may throw std::bad_alloc - if so, pointers_m is unchanged
        auto ptr = std::make_shared<A_Type>(std::forward<Args>(args)...);
        pointers_m.emplace_back(ptr);
        return ptr;
    }

    void remove(const size_t& key)
    {
        pointers_m.erase(pointers_m.begin() + key);
    }

    // Will throw std::out_of_range if key is invalid
    std::weak_ptr<A_Type> operator[](const size_t& key)
    {
        return pointers_m[key];
    }
};

At this point, there's almost nothing here that's not in the underlying container, and much that's missing (reserve(), iterators, member types, and more).  What you end up with is a convenience function for creating a smart-pointer member in a collection:

#include <memory>

template<typename Container, class... Args>
std::weak_ptr<typename Container::value_type> push(Container& c, Args&&... args)
{
    // may throw std::bad_alloc - if so, c is unchanged
    auto p = std::make_shared<typename Container::value_type>(std::forward<Args>(args)...);
    c.emplace_back(p);
    return p;
}

You might be able to further generalize that to not depend on emplace_back as the insertion method and/or use Concepts to constrain the function and give more informative errors if it's misused.

I'm sorry if the above reduces what you've written to something near-trivial, but I hope you've learnt a lot whilst you were doing it (and that you can apply that to your future projects).  Certainly, you seem to have your head around forwarding references, which is more than many can say!


Binding data in a TemplateField This is my current RowDataBound function.  I think this functionality is a bit horrrendous really and it winds up with an extreme amount of LINQ SQL queries being passed around (which makes for big server load etc).

What I'm wondering is, is there any way that I could pull all the data in the first select statement, put it somewhere, and then have the fields in the templatefields bind back to this instead of running a query for every single row?

protected void GridView1_RowDataBound(object sender, GridViewRowEventArgs e)
    {

        if (e.Row.RowType == DataControlRowType.DataRow)
        {
            HiddenField hidField = (HiddenField)e.Row.FindControl("dbID");
            Table tbldetails = new Table();
            TableHeaderRow tblheaderrow = new TableHeaderRow();
            TableHeaderCell tblheadercell = new TableHeaderCell();
            tblheadercell.Text = "Details";
            tblheaderrow.Cells.Add(tblheadercell);
            tbldetails.CssClass = "table";
            int rowID = Convert.ToInt32(hidField.Value);
            Panel pnl = (Panel)e.Row.FindControl("deviceDetails");
            bool altField = false;

            var query = (from q in db.GetTable<priceData>()
                         where q.ID == rowID
                         select q).FirstOrDefault();
            var values = typeof(priceData)
                .GetProperties()
                .Select(p => p.GetValue(query, null))
                .ToArray();

            foreach (var value in values)
            {
                TableCell cell = new TableCell();
                TableRow tr = new TableRow();
                cell.Text = value.ToString();
                tr.Cells.Add(cell);
                tbldetails.Rows.Add(tr);
            }
            pnl.Controls.Add(tbldetails);
        }

EDIT

To explain myself a bit better, what I have is a GridView with rows that are expandable. In the expandable row under datarow (which lives in a TemplateField) I want to put all the data from the datarow in the sqldatasource. To me, this means I would have to have a collection of some sort to iterate through, to add to the template field. That said I could be totally wrong and I could be going about this the wrong way. Am I just totally barking up the wrong tree?

My main problem with this code is that it feels "rambling". Things happen in the right order, but it's all mangled. For instance:

You retrieve hidField on line 6 and then don't use it until line 13.
You retrieve pnl on line 14 and then don't do anything with it until line 33.
You create tbldetails on line 7 but assign its property CssClass only on line 12 (and "table" is IMHO a bad css class name).

Etcetera. Group these assignments so the code become more logical, so there's more of a flow.

Variable altField isn't used at all.

I'm not a big fan of mixing LINQ styles, and I find the SQL-like version often too verbose. Example:

(from q in db.GetTable<priceData>()
where q.ID == rowID
select q).FirstOrDefault();

... can be rewritten as:

db.GetTable<priceData>().FirstOrDefault(x => x.ID == rowID);

priceData is a class name, so why then is it camelCase instead of PascalCase?

Why not use var instead of repeating the class name like here:

TableHeaderRow tblheaderrow = new TableHeaderRow();
TableHeaderCell tblheadercell = new TableHeaderCell();
Table tbldetails = new Table();

Compound words should have a capital at each compounded word, so it should be tblHeaderRow instead of tblheaderrow etc.

Avoid abbreviations. Why hidField and not hiddenField?

This is "old style" C#:

TableHeaderCell tblheadercell = new TableHeaderCell();
tblheadercell.Text = "Details";

These days I'd expect to see this:

var tableHeaderCell = new TableHeaderCell
{
   Text = "Details"
};

Now, WRT your approach: it depends on the volume of data. If this detail table isn't that "heavy" and the main table isn't that big, I would consider filling it up for each row by default and toggling its visibility through client-side code (JavaScript/JQuery + CSS). Not that in this case you should NOT retrieve it ID by ID, but instead in one query.

If that's too much data to send to the browser in one go, then I'd consider your way. There really is no "one size fits all" solution, it all depends on the volume of data and what your users expect from your UI.


Sorting a list but the sort order isn't known until runtime I need to sort a list of strings that I'm reading in but the sorted order of that list is not predetermined (I can't hard code it); I read in the sort order from a source and build a comparator by assigning a numerical value to each entry in a map and then use that to determine the order.
My implementation is pretty simple and straightforward. I don't know if there is any room to optimize this but given that sorting is generally a slow process in itself, I wanted to submit this for code review in case there are in fact any improvements to be made.
The custom comparator will be cached after creation so I won't be building a new one unless required to do so.
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;

public class RunTest {
   // Runnable test
   public static void main(String[] args) {
      CustomComparator comparator = CustomComparator.fromList(getSortOrder());

      List<String> list1 = getTestList();

      System.out.println("Test list: \n  " + list1);

      Collections.sort(list1, comparator);

      System.out.println("Test list (sorted): \n  " + list1);
   }

   // Comparator
   public static class CustomComparator implements Comparator<String> {
      private Map<String, Integer> _sortOrder;

      public CustomComparator(Map<String, Integer> sortOrder) {
         _sortOrder = sortOrder;
      }

      @Override
      public int compare(String l, String r) {
         Integer lvalue = _sortOrder.get(l);
         Integer rvalue = _sortOrder.get(r);

         if (lvalue == null || rvalue == null)
            return 0;

         if (lvalue < rvalue)
            return -1;

         return 1;
      }

      public static CustomComparator fromList(List<String> sortOrder) {
         // Map each list item to a comparable integer
         Map<String, Integer> customSortOrder = new HashMap<>();

         int i = 0;

         for (String s : sortOrder)
            customSortOrder.put(s, i++);

         return new CustomComparator(customSortOrder);
      }
   }

   // Custom sort order
   public static List<String> getSortOrder() {
      List<String> customSortOrder = new LinkedList<>();

      customSortOrder.add("c");
      customSortOrder.add("b");
      customSortOrder.add("a");

      return customSortOrder;
   }

   // Test list
   public static List<String> getTestList() {
      List<String> randomList = new LinkedList<>();

      randomList.add("b");
      randomList.add("a");
      randomList.add("c");

      return randomList;
   }
}

Output:

Test list:
  [b, a, c]
Test list (sorted):
  [c, b, a]

This:
     if (lvalue == null || rvalue == null)
        return 0;

doesn't seem very safe. It says that if either of the values does not have a defined order, then don't care about the order, consider them equivalent. This will have the effect of interleaving such items throughout your sorted list.
Technically this fails the requirement of Comparator that

The implementor must also ensure that the relation is transitive

In other words, given some strings a, b and c with ordering indices 1, null and 2, your existing implementation will say that a == b, b == c but a != c - this is non-transitive.
Instead: either throw an exception if there is a value with an undefined order; or choose to default undefined-order values at the beginning or end of the list.


Loading sound effects into an array I'm loading my game's sound effects into an array to easily pick a random one and play it. Previously, I used a switch statement to select a random sound but it looks less verbose to just pick an array key.
Is it inefficient to use an array to store and play my sound effects? Is there a better way to do this or any improvements I can make to my implementation?
let sounds = [];

function loadSounds() {
  const sources = ['sound1.ogg', 'sound2.ogg', 'sound3.ogg', 'sound4.ogg', 'sound5.ogg', 'sound6.ogg'];

  for (let i = 0; i < sources.length; i++) {
      sounds[i] = new Audio(sources[i]);
  }
}

function playRandomSound() {
    const randomNumber = Math.floor(Math.random() * 6);

    sounds[randomNumber].play();
}

An array is perfectly fine, it's what I'd prefer as well. But there are some improvements you can make:

When you have an array and you want to create a new array by transforming every item of an old array, it's most appropriate to use Array#map:
// I'd call it `filenames` and `audios`, not `sources` and `sounds`;
// `audios` is plural of Audio and is a bit more precise
const audios = filenames.map(filename => new Audio(filename));

If your sound files are really numeric-indexed, it would be even easier to just iterate over the numbers 1 to 4, rather than having to list sound1.ogg, sound2.ogg, etc.

Always declare variables with const when possible - only use let when you must reassign the variable name. Using const makes code easier to read, when a reader can determine that the variable name won't ever be reassigned at a glance.

At the moment, if playRandomSound is called multiple times in a row, multiple sounds may be active at once, which may sound very discordant and not be desirable. In such a case, you may wish to save the randomNumber in a persistent outer variable so that you can call .stop() on the prior Audio before .play()ing another. (Or, if you want to have playing sounds overlap, your current approach is just fine)

If your sources array's length changes, you'll also need to change this section: Math.floor(Math.random() * 6); Better to put the array's length there instead of the 6.

const audios = Array.from(
  { length: 6 },
  (_, i) => new Audio(`sound${i + 1}.ogg`)
);
function playRandomSound() {
    const randomNumber = Math.floor(Math.random() * audios.length);
    sounds[randomNumber].play();
}


array to return on conditions I am writing a method to find me a price range for special price as well as the retail price. Such as in example below I have price range for retail from 129 to 329 where as special price range is 89 to 199

array
0 =>
    array
            'id' => 83739000038374
            'special-price' => 99.99
            'retail-price' => 199
1 =>
    array
            'id' => 83739000038374
            'special-price' => 199.99
            'retail-price' => 299
2 =>
    array
            'id' => 83739000038374
            'special-price' => 89
            'retail-price' => 129
3 =>
    array
            'id' => 83739000038374
            'special-price' => 179
            'retail-price' => 329

my code is

public function getPriceRange(){
    $priceRange = array();
    $showRange = true;
    if($originalArray && count($originalArray) > 1){
        foreach($originalArray as $idx => $originalArraySingle){
            if($idx == 0){
            $priceRange['price']['max'] = $originalArraySingle['price'];
            $priceRange['price']['min'] = $originalArraySingle['price'];
            $priceRange['original-price']['max'] = $originalArraySingle['original-price'];
            $priceRange['original-price']['min'] = $originalArraySingle['original-price'];
            }else{
                if($originalArraySingle['price'] > $priceRange['price']['max'] ){
                    $priceRange['price']['max'] = is_int($originalArraySingle['price']) ? number_format((float)$originalArraySingle['price'], 2, '.', '') : $originalArraySingle['price'];
                }
                if($originalArraySingle['price'] < $priceRange['price']['min'] ){
                    $priceRange['price']['min'] = is_int($originalArraySingle['price']) ? number_format((float)$originalArraySingle['price'], 2, '.', '') : $originalArraySingle['price'];
                }
                if($originalArraySingle['original-price'] > $priceRange['original-price']['max'] ){
                    $priceRange['original-price']['max'] = is_int($originalArraySingle['original-price']) ? number_format((float)$originalArraySingle['original-price'], 2, '.', '') : $originalArraySingle['original-price'];
                }
                if($originalArraySingle['original-price'] < $priceRange['original-price']['min']){
                    $priceRange['original-price']['min'] = is_int($originalArraySingle['original-price']) ? number_format((float)$originalArraySingle['original-price'], 2, '.', '') : $originalArraySingle['original-price'];
                }
            }
        }
        //doesnt make sense to show price range if its same.
        if($priceRange['price']['max']===$priceRange['price']['min'] ||  $priceRange['price']['max']===$priceRange['price']['min']){
            $showRange = false;
        }
    }
    return $showRange ? $priceRange : null;
}

I am looking for scalability and for fastest code processing.

if($originalArraySingle['original-price'] < $priceRange['original-price']['min']){
    $priceRange['original-price']['min'] = is_int($originalArraySingle['original-price']) ? number_format((float)$originalArraySingle['original-price'], 2, '.', '') : $originalArraySingle['original-price'];
}

You could write a function for this. And one for maximize.

protected function minimize($a,&$b){
    if($a<$b){
        $b = is_int($a)?number_format((float)$a,2,'.',''):$a;
    }
}

I would also write.

if($priceRange['price']['max']===$priceRange['price']['min'] ||  $priceRange['price']['max']===$priceRange['price']['min']){
        return null;
    }

and change

return $showRange ? $priceRange : null;

to

return $priceRange;

so you have one variable less.


How can I use object oriented principles in the following scenario? I have 3 car rental agencies.  Each agency requires the data in XML, but in a different format, for example:

Agency 1

<Rental>
<Customer>
<FirstName>test</FirstName>
<LastName>test</LastName>
</Customer>
<Pickup date="07/20/2012"/>
<Dropoff date="07/25/2012"/>
<Deposit cost="100"/>
</Rental>

Agency 2

<Rental>
<Customer>
<FirstName>test</FirstName>
<LastName>test</LastName>
</Customer>
<Pickup>07/20/2012</Pickup>
<Dropoff>07/25/2012</Dropoff>
<Deposit>100</Deposit>
</Rental>

Agency 3

<Rental pickup="07/20/2012" dropoff="07/25/2012" deposit="100">
<Customer>
<FirstName>test</FirstName>
<LastName>test</LastName>
</Customer>
</Rental>

As you can see from above, all 3 basically contain the same information, altough this doesn't have to be the case (some can contain more or less information), but it is structured different, so the way I access it is different.  What is the best approach to take so I can write the most minimum code, but be able to adapt to new rental agencies that come along with a different structure?

Right now, I am doing something like this:

public class Agency1
{
SubmitRental()
{
//Parse XML for Agency 1
}

//Other methods for agency 1
}

public class Agency2
{
SubmitRental()
{
//Parse XML for Agency 2
}

 //Other methods for agency 2
}

public class Agency3
{
SubmitRental()
{
//Parse XML for Agency 3
}

//Other methods for agency 3
}

In the above, the classes contain the same methods, but the way they are implemented is different.  There are some methods, properties, etc that are in some classes, but not in others.  Are interfaces the best way to approach this?  If so, should everything be made an interface?

In the XML samples above, the data was the same, but the format was different, which led some to bring up mapping the all the different formats to a set of common classes, but what about the scenario where not only is the format different, but the data is different as well?

For example:

Agency 4

<Rental start="07/20/12" end="07/25/12">
<Name>John Doe</Name>
<DriverLicenseNumber>193048204820</DriverLicenseNumber>
<DOB>3/4/64</DOB>
<Rental>

Assuming all three companies require the same information, create a base class "CarRentalInformation."  This information will store/transmit all pertinent information.

Create serializer/deserializer classes for all types of can rentals you need.  You could probably use the factory pattern effectively here even if you need specialized information for each type you need.

The logic could then be written as

public List<CarRentalInformation> GetCarRentalInformation(string XML)
{
   CarRentalXmlType xmlType = this.DetectCarRentalXmlFormat(XML);
   ICarRentalFactory deserializer = this.GetCarRentalFactory(xmlType);

   return deserializer.Deserialize(XML);
}

public string GetCarRentalXML(List<CarRentalInformation> rentalInfo, CarRentalXmlType xmlType)
{
   ICarRentalFactory serializer = this.GetCarRentalFactory(xmlType);
   return serializer.Serialize(rentalInfo);
}

private CarRentalXmlType DetectCarRentalXmlFormat(string XML)
{
   //do some magic detectivework, and return appropriate type
   return CarRentalXmlType.Budget;
}

private ICarRentalFactory GetCarRentalFactory(CarRentalXmlType xmlType)
{
   switch(xmlType)
   {
       case CarRentalXmlType.Budget:
          return new BudgetCarRentalFactory();
       case CarRentalXmlType.Dollar:
          return new DollarCarRentalFactory();
       default
          return null;
    }
}

public interface ICarRentalFactory
{
    public List<CarRentalInformation> Deserialize(string XML);
    public string Serialize(List<CarRentalInformation> rentalInfo);
}

public class BudgetCarRentalFacotry : ICarRentalFactory
{
   //contains code to serialize/deserialize XML into CarRentalInformation objects
}


Getting familiar with OOP - Factories I have no idea if I'm doing this efficiently, or ideally, but I've written what I think is a factory. Please help me understand if this is good practice, or if there is another direction I should take this. There's no particular project in mind with this, I'm just farting around. (Written in PHP)

class Factory
{
    private function __construct(){}

/**
 * @param $obj    Object to Create.
 * @param $params Params (use array for multiple parameters) to be passed.
 */
    public static function build($obj, $params=null) {
        switch ($obj) {
            case 'db': return new DB($params);
            default:   return null;
        }
    }
}

This lets me do something like $db = Factory::build("db","test_database_name"); The DB class just builds a PDO connection using appropriate parameters based on the database I'm trying to access, which is useful for me because I deal with MySQL and MS SQL all day long, and I don't want to have to remember all the connection strings, users, and passwords (stored in the DB class). That's kind of besides the point, I just intend this to be expanded as I develop more useful classes, so...

I just want to get some opinions on the the topic of a Factory. Is this a good method? are there others that are considered superior? If so, what have I overlooked?

In my opinion, a factory should create a specific class, global ones like this one shouldn't be used. For instance, having a PizzaFactory is something like this:

class PizzaFactory
{
    public static function createPizza($type)
    {
        switch ($type) {
            case 'salami':
                return new SalamiPizza();
                break;

            case 'hawai':
                return new HawaiPizza();
                break;
        }
    }
}

  I just want to get some opinions on the the topic of a Factory. Is this a good method? are there others that are considered superior? If so, what have I overlooked?

An upcomming design pattern is depedency-injection. Just search for some resoures about that and keep reading. It is used in all modern frameworks (ZF2 and Symfony 2 uses it).

See also this article serie by Fabien Potencier: "What is Dependency Injection?"


Most common words in a text file I've made a program that outputs you the most common word in txt file. Does anybody know how to optimize it so that it would work for bigger files and run faster?

#include <iostream>
#include <string>
#include <fstream>
#include <cstdlib>
#include <vector>
#include <algorithm>
#include <math.h>

using namespace std;

int main()
{
    ifstream in("file.txt");

    if(!in){
        cerr << "Could not open file.txt.";
        return EXIT_FAILURE;
    }

    string str, str2, strn, tab[10000], tab2[10000];
    int i, k, j, n, l, tabl;
    char c = 179;
    vector<int> tabs;
    vector<string> stringi;

    while(getline(in, str2)){
        str += str2;
        str += ' ';
    }
    k = 0;
    for(i = 0; i < str.length(); i++){
        if(str[i] != ' ' && str[i] != '.' && str[i] != '\t' && str[i] != ','
           && str[i] != ';' && str[i] != ':' && str[i] != '}' && str[i] != '{'){
            tab[k] += tolower(str[i]);
        }else{
            k++;
        }
        if(str[i] == '.' || str[i] == '\t' || str[i] == ',' || str[i] == ';'
        || str[i] == ':' || str[i] == '}' || str[i] == '{') {
            k--;
        }
    }
    tabl = k;

    k = 0;
    for(i = 0; i < tabl; i++){
        for(j = 0; j < tabl; j++){
            if(tab[i] == tab[j]){
                k++;
            }
        }
        tabs.push_back(k);
        k = 0;
    }
    for(i = 0; i < tabl; i++){
        for(j = 0; j < tabl-1; j++){
            if(tab[j] < tab[j+1]){
                n = tabs.at(j);
                tabs.at(j) = tabs.at(j+1);
                tabs.at(j+1) = n;
                strn = tab[j];
                tab[j] = tab[j+1];
                tab[j+1] = strn;
            }
        }
    }
    for(i = 0; i < tabl; i++){
        for(j = 0; j < tabl-1; j++){
            if(tabs.at(j) < tabs.at(j+1)){
                n = tabs.at(j);
                tabs.at(j) = tabs.at(j+1);
                tabs.at(j+1) = n;
                strn = tab[j];
                tab[j] = tab[j+1];
                tab[j+1] = strn;
            }
        }
    }
    tab2[0] = tab[0];
    for(i = 0; i < tabl; i++){
        if(tab[i] != tab[i+1]){
            tab2[i] = tab[i+1];
        }
    }
    k = 1;
    l++;
    for(i = 0; i < tabl; i++){
        if(!tab2[i].empty()){
            l++;
        }
    }
    cout << "------------------------------------" << endl;
    cout << "|--->TABLE OF MOST COMMON WORDS<---|" << endl;
    cout << "------------------------------------" << endl;
    for(i = 0; i < tabl; i++){
        if(!tab2[i].empty() && k <= 20 ){
            cout << c << k++ << "." << '\t' << c << tab2[i] << '\t' << c << "*" <<
            tabs.at(i+1)
            << '\t'  << c << roundf(((float)tabs.at(i+1)*100/l)*100)/100 << "%" <<
            endl;
        }
    }
    cout << "------------------------------------" << endl ;
    cout << "|----->Dif. strings: " << '\t' << l << "<-------|" << endl ;
    cout << "------------------------------------" << endl;

    return 0;
}

Output image:

Declare variables as late as possible. int i, k, j, n, l, tabl; What do these variables do? It is quite difficult to figure that out. If you declare and initialize them when needed it would be clear.
Keep variables as local as possible. A loop such as for(i = 0; i < str.length(); i++) makes me think the result of i is required later which it is not (I think). Prefer for (int i = 0; i < str.length(); i++).
People do not like using namespace std;.
tab[10000] and char c = 179; have magic numbers. Why 10000 and 179? Since you know how to use vector it is hard to guess why you thought having 10000 stings twice is a good idea.
This seems to be a major performance problem (you are supposed to profile because obvious performance problems can get fixed by the optimizer and the line between what the optimizer thinks is obvious and which is not is very hard to predict):

while(getline(in, str2)){
    str += str2;
    str += ' ';
}

This produces a string to hold a single line. Then it adds a line, but it is out of memory, so it allocates new memory for 2 lines, copies 2 lines and throws the memory for 1 string away. Then it allocates memory for 3 lines, copies 3 lines and throws 2 lines of memory away. When you start allocating memory for 100 lines, copy 100 lines and throw 99 lines worth of memory away things get slow. I would suggest counting words one line at a time instead of reading the whole file at once.
Use functions. Some people do not allow more than 10 lines per function, otherwise it must be re-factored into multiple functions. 10 lines are a bit strict, but you should separate concerns. One function to get the input, one function to do the word counting, one function to do the output. Maybe some functions will be too complicated and need to be split up further. The point is that if you need to change a part of the program you see from the function names where to change what.
I looked at your loops and indexes and I don't even want to try to understand what is going on. You can replace almost all of that with a map<string, int>.

This is what I came up with (it is 4:35 am here). My code kind of deserves a review of its own because it makes me look a bit hypocritical.

    #include <iostream>
    #include <string>
    #include <map>
    #include <algorithm>
    #include <cctype>
    #include <vector>
    #include <sstream>
    #include <fstream>
    #include <iomanip>

    void removeNonAlphabeticalCharacters(std::string &s){
        s.erase(std::remove_if(std::begin(s), std::end(s), [](char c){ return !isalpha(c); }), std::end(s));
    }

    template<class Iterator>
    //constraint: Iterator::value_type == std::pair<std::string, int>
    void displayRanking(Iterator begin, Iterator end, int wordcount){
        int index = 1;
        while (begin != end){
            std::cout << '|' << index << ".\t|" <<
                begin->first << "\t|" << begin->second << "\t|" <<
                std::fixed << std::setprecision(2) << begin->second * 100.f / wordcount << "%\n";
            ++begin;
            index++;
        }
    }

    int main(){
        std::map<std::string, int> wordcounter;
        std::string input;
        int totalwords = 0;
        std::ifstream in("file.txt");
        while (in >> input){
            removeNonAlphabeticalCharacters(input);
            wordcounter[input]++;
            totalwords++;
        }
        if (totalwords == 0){
            std::cout << "failed reading words from file\n";
            return 0;
        }

        //how many words to display in the ranking
        const int top = std::min((size_t)20, wordcounter.size());
        //copy wordcounter to vector for sorting
        std::vector<std::pair<std::string, int>> toplist;
        toplist.reserve(wordcounter.size());
        std::copy(std::begin(wordcounter), std::end(wordcounter), std::back_inserter(toplist));
        auto comparator = [](const std::pair<std::string, int> &lhs, const std::pair<std::string, int> &rhs){return lhs.second > rhs.second; };
        std::partial_sort(std::begin(toplist), std::begin(toplist) + top, std::end(toplist), comparator);

        displayRanking(std::begin(toplist), std::begin(toplist) + top, totalwords);
    }

There are many ways to improve readability and performance still, but let me know how long my code takes with your input file.


Simple multi-threaded time scheduler Here's a rather hastily made time scheduler to suit my requirements and even for my future projects. It is working as intended but I want to improve more . I am a self taught enthusiast, and rather weak as a language lawyer.

This scheduler should be able to schedule tasks which will be executed the task at the right instance. It should arrange the time-points in chronological(//TODO boundary condition) order.

typedef std::function<void(void)> func;
typedef std::chrono::system_clock::time_point time_point;
std::map<time_point, func> schedule;

int main()
{
    schedule.insert(make_pair(std::chrono::system_clock::now() + std::chrono::seconds(4s), []() { std::cout << " Slow first \n"; }));
    schedule.insert(make_pair(std::chrono::system_clock::now() + std::chrono::seconds(3s), []() { std::cout << " Medium second \n"; }));
    thread t([]()
    {
        while (!schedule.empty())
        {
            if (chrono::system_clock::now() >= (schedule.begin())->first)
            {
                (schedule.begin())->second();
                schedule.erase(schedule.begin());
            }
            this_thread::sleep_for(chrono::milliseconds(1ms));
        }
    });
    schedule.insert(make_pair(std::chrono::system_clock::now() + std::chrono::milliseconds(500ms), []() { std::cout << " Impatience third \n"; }));
    std::cout << " main is waiting for the thread to end already \n";
    t.join();
    return 0;
}

I notice a very bad (anti-)pattern that you're using. You're polling. "Wake up frequently to check if there's any work, if not go back to sleep". The thread demands to be woken up every 1ms or so. This will cause a big burden on the OS thread scheduler and hinder performance. It also prevents your CPU from going to sleep (low power mode) so the power usage and heat generated by your CPU will greatly increase.

What you should do instead is to have your thread stay asleep until the next time that an event is scheduled to occur. Make sure your sleep has a method of being interrupted in case a new event comes in. Look up "condition variables". (#include <condition_variable>)

Secondly, you're treating std::map as if it's thread-safe. It's not. If the main thread is modifying the map at the same instant that the scheduler thread is checking it, bad things can happen. Spooky bugs that occur intermittently and are impossible to recreate or trace. You need to use a thread-safe data structure (or control access using some kind of locking mechanism, which you'll need anyway for the condition variable) if you're going to share it between multiple threads.

Let me make it clear: don't expose the actual std::map to the event providers. All access must go through methods that enforce proper object locking (and wake up the thread if necessary).

If you want to be able to expand this, you really need to make it its own class with a specific interface -- then you can define the implementation however you wish. The operations could be: add an event, remove an event, scheduling on, scheduling off, list all events, enumerate events one by one, change time of an event, change action of an event, remove all events.


Platform independent whereis function I'm trying to write a platform independent Python (>=3.6.5) package that has a handful of java .class files that need to compiled using javac and run using java. This is simple enough task but I need to know if user has these installed already and the location of the executable assuming that neither of them is on the system path. That way I can just call the executable by it's path using subprocess.run. Which bring me to the subject of this review a platform independent whereis command.

import os, subprocess
from subprocess import CalledProcessError

def whereis(app):
    """Return a list of locations a given application.

    Relies on the `where` system command in Windows and the `whereis` command in Unix.

    Parameters
    ----------
    app : str
        Can be any application on the system path e.g. java.

    Returns
    -------
    result : list
        A list of locations where applications is installed.

    Useage
    ------
    >>>whereis('javac')
    'C:\\Program Files\\Java\\jdk1.8.0_162\\bin\\javac.exe'
    """

    result = None

    if os.name == "nt":# Windows
        try:
            result = subprocess.check_output("where {}".format(app))

        except CalledProcessError as err:
            print("Application ,",app,", not forund.",err)

    else:# Unix
        try:
            result = subprocess.check_output("whereis {}".format(app))

        except CalledProcessError as err:
            print("Application ,",app,", not found.",err)

    if result is None:
        print("")
        result = []
        return result

    else:
        result = result.decode().replace("\r", "").split("\n")
        result = list(filter(lambda x: len(x)>0, result))
        return result

Questions

Is there a standard library function that already covers this? I couldn't find one but that doesn't mean anything.
Are there any caveats or edge cases that I'm missing?
Can any general improvements be made to the code or docstring?

A couple of small things:

If you're specifically looking for executable files, you might use which instead of whereis command. From this answer, you can see the difference between the two using whatis:

$  whatis which
which                (1)  - shows the full path of (shell) commands

$  whatis whereis
whereis              (1)  - locate the binary, source, and manual page files for a command

Be consistent. You've used two different formatting styles:

"where {}".format(app)

and

"Application ,",app,", not forund.",err  # you also have a spelling mistake here (forund -> found)

I'd recommend you use the former one
You should have at least two spaces before an inline comment
You should put a space after the comma in almost every situation. (e.g: print("Application,",app,", not found.",err) -> print("Application,", app, ", not found.", err))


Extracting all-caps variables from string expressions I'm extracting variables from user entered mathematical expressions, so that I can enter them into the expression evaluator exprtk. For now, the condition is given that variable names will be composed of consecutive capital letters. Any mathematical constants or function names will be lowercase. Some hypothetical inputs and expected outputs:

Input: "(A + B) - 2"
Expected Output: { A, B }

Input: "pow(AC,E) * ( E * F )"
Expected Output: { AC, E, F }

The code I have right now seems to work, but I would love a critique. I add a space to the end of the string to make my algorithm catch cases at the end of the string, which seems a bit hacky(?).

template<class T>
std::vector<std::string> MapEvaluator<T>::extractVariables(std::string expression) {

    expression = expression + " ";
    std::vector<std::string> variables = {};
    std::string substring;
    size_t consecutiveCharsCaps = 0;
    bool previousCharCaps = false;

    for (size_t i = 0; i < expression.length(); i++)
    {
        if (isCapital(expression[i]))
        {
            consecutiveCharsCaps++;
            previousCharCaps = true;
        }
        else {
            if(previousCharCaps) {
                substring = expression.substr(i - consecutiveCharsCaps, consecutiveCharsCaps);
                variables.push_back(substring);
                consecutiveCharsCaps = 0;
                previousCharCaps = false;
            }
        }
    }

    unique(variables);
    return variables;
}

template <class T>
void MapEvaluator<T>::unique(std::vector<std::string> &vec)
{
    auto end = vec.end();
    for (auto it = vec.begin(); it != end; ++it) {
        end = std::remove(it + 1, end, *it);
    }
    vec.erase(end, vec.end());
}

template<class T>
bool MapEvaluator<T>::isCapital(char c) {
    return (c >='A' && c <= 'Z');
}

Some observations:

It seems that std::adjacent_find essentially already does what you want, which is extracting all continuous sub-strings consisting of uppercase letters.
Removing duplicates from a vector might be fine, but you can also avoid this completely by inserting the found elements into a std::set. I suspect that the number of unique variables is always small, so this is a cleaner approach.
There is no reason for isCapital to be a member function. Instead, it should be a free function. Remember that interfaces should be complete but minimal. But in fact, there's no reason for the function in the first place: the standard already has std::isupper that we should rather use.

So with these in mind, we can re-write your function to e.g.,:

std::vector<std::string> get_variables(const std::string& str)
{
    std::set<std::string> vars;

    for (auto first = str.cbegin(); first != str.cend(); )
    {
        auto var_end = std::adjacent_find(first, str.cend(),
            [](char a, char b)
        {
            return std::isupper(static_cast<unsighed char>(a)) !=
                std::isupper(static_cast<unsigned char>(b));
        });

        if (var_end != str.cend())
        {
            ++var_end;
        }

        if (std::isupper(static_cast<unsigned char>(*first)))
        {
            vars.insert(std::string(first, var_end));
        }

        first = var_end;
    }

    return std::vector<std::string>(vars.cbegin(), vars.cend());
}

Notice that we've cast char to unsigned char before the call to std::isupper (thanks to Deduplicator for explaining this to me). The reason is that std::isupper takes an argument of type int which must be in the range of unsigned char (or special value EOF, -1). Unfortunately, a plain char has either the range of signed char or unsigned char, so we perform a cast to confer to this common convention of character classification functions.


GUID Generation and Invalidation I'm updating an old ECS I wrote a couple years back, and I've written the following piece of code in order to generate a GUID:

const auto& es::EntityStore::RequestEntity()
{
    if(m_freeEntities.size() != 0)
    {
        return ec::ENTITY_ID_INVALID;
    }
    //Get the lastmost EntityID
    auto eid = m_freeEntities.back(); //an std::vector<uint64_t>
    m_freeEntities.pop_back();
    //Get the index value of the EntityID
    uint32_t idx = 0;
    idx = eid & 0XFFFFFFFF;
    //Get the version value of the EntityID
    uint32_t version = 0;
    version = ((eid >> 32) & 0xFFFFFFFF) + 1;

    //Update the metadata at idx
    m_entityMetadata.at(idx).version = version;

    //Create & return the new EntityID
    eid = version;
    eid = eid << 32 | idx;
    //an unordered_set<uint64_t>
    return *m_livingEntities.insert(eid).first;
}

However, I believe it can be written out nicer, and perhaps with less machine code generated.

HOW THE GUID WORKS

I am using a uint64_t to represent my GUID. Reading left-to-right, the first 32 bits will represent the version of the GUID, while the last 32 bits represent the index of the GUID.

The index will be used to lookup items in other arrays and vectors, so it never changes.

The version, however, is used to differentiate between an object who might have the same index as another.

When a GUID is invalidated, it is stuck onto the end of a vector.
When it is reused, it's version goes up by a counter of 1.

So, if there are any game objects out there referencing ID 0 or belonging to ID 0, even though it occupies the same spot in the array because of it's index, it's GUID will be different because it's version is 1 higher than whatever GUID came before it.

What I see here:

1.

You use autos, whose bit length is unknown in compile time. Despite that, the code uses them as 64-bit values. Afaik you should have some structural guarantee that they are really 64-bit (f.e. instead of "auto" use simply uint64_t).

2.

This code:

uint32_t version = 0;
version = ((eid >> 32) & 0xFFFFFFFF) + 1;

could be simplified to

uint32_t version = ((eid >> 32) & 0xFFFFFFFF) + 1;


ImportExcel for Dataset with Multiple tables I need to be able to create an Excel document with multiple tabs for a dataset with multiple results\data tables. I have a stored procedure with 3 selects in them, fairly simple ones pulling data from the sys tables in SQL Server.

Below is my script where I am assuming there will be 5 tables, if there are more then it will be missed and if there are less and then the other tabs will be empty.

What's the best way to loop through the dataset and create the Excel document?

$SQLServer = '.\SQLEXPRESS';
$Database = 'AdventureWorks';
$sqlCommand = "SELECT 'Tables' AS DataSetName, * FROM sys.tables

SELECT 'Columns' AS DataSetName, * FROM sys.columns";

$connectionString = "Server=$SQLServer;Database=$Database;Integrated
Security=true"
$connection = New-Object
System.Data.SqlClient.SqlConnection($connectionString)
$command = New-Object System.Data.Sqlclient.Sqlcommand($sqlCommand,
$connection)
$connection.Open()
$adapter = New-Object System.Data.sqlclient.SqlDataAdapter $command
$dataset = New-Object System.Data.DataSet
$adapter.Fill($dataSet) > null
$connection.Close()

foreach($table in $dataset.Tables)
{
    Write-Host $table.TableName
    if ($($table.TableName) -eq 'table'){$Table1 = $table}
    if ($($table.TableName) -eq 'table1'){$Table2 = $table}
    if ($($table.TableName) -eq 'table2'){$Table3 = $table}
    if ($($table.TableName) -eq 'table3'){$Table4 = $table}
    if ($($table.TableName) -eq 'table4'){$Table5 = $table}
}

$DataToGather = @{
    table = {$Table1}
    table1 = {$Table2}
    table2 = {$Table3}
    table3 = {$Table4}
    table4 = {$Table5}
}

Export-MultipleExcelSheets -Show -AutoSize 'C:\Temp\Test.xlsx' $DataToGather

$DataToGather.Clear()

If the Excel sheet names should retain the original TableNames this could do:
(can't test myself)
The code contains two commented out alternatives.

# your code upto $connection.Close() ...

$DataToGather = @{}
foreach ($table in $dataset.Tables) {
    Write-Host $table.TableName
    Add-Member -InputObject $DataToGather -Name $table.TableName -Value $table
#   Add-Member -InputObject $DataToGather -Name $table.TableName -Value $dataset.Tables['$($table.TableName)']
#   $DataToGather.Add($table.TableName,$table)
}

Export-MultipleExcelSheets -Path 'C:\Temp\Test.xlsx' -InfoMap $DataToGather -Show -AutoSize
#$DataToGather

$DataToGather.Clear()


Finding and moving the next obstacle in a player's path This code finds the next obstacle in the player's path, then moves it. I am trying to make it more efficient, because this code runs for each frame, and it lags a bit. It does work though.

There are 8 blockers, which are basically obstacles. We subtract from the players rotation until the rotation is equal to one of these eight blockers. Then we move the blocker.  Each blocker cannot be moved two times in a row, though.

You might be wondering why rotation determines if the player collides. That is because my game is based around a planet, to where the player rotates.

whileLoop:
    while (true) {

        for (Blocker blocker : blockers) {
            if (playerRotation == (int) blocker.getRotation()) {
                if(lastBlockerMoved != blocker) {
                    lastBlockerMoved = blocker;
                    float gap = Tools.generateFloat(Config.minBlockerGap, Config.maxBlockerGap);
                    float y = Tools.generateFloat(Config.minBlockerY, Config.maxBlockerY - gap);
                    float duration = Tools.generateFloat(Config.minBlockerYDuration, Config.maxBlockerYDuration);
                    blocker.addAction(Actions.moveTo(blocker.getX(), y, duration, Config.blockerInterpolation));
                    blocker.addAction(GapSizeAction.getGapSizeAction(gap, duration));
                }

                break whileLoop;
            }
        }

        playerRotation--; //-- is going right, ++ would be going left
        while (playerRotation > 360) {
            playerRotation -= 360;
        }
        while (playerRotation <= 0) {
            playerRotation += 360;
        }
    }

Take out the variables that are not changing within the loops and initialise them once.

float gap = Tools.generateFloat(Config.minBlockerGap, Config.maxBlockerGap);
float y = Tools.generateFloat(Config.minBlockerY, Config.maxBlockerY - gap);
float duration = Tools.generateFloat(Config.minBlockerYDuration, Config.maxBlockerYDuration);

It is unnecessary to continue to break out of the for loop to change the playerRotation. so then add within your for loop the playerRotation decrement with two if conditions (the while is unnecessary, it can only change by 1, so cannot exceed greater than 360 or less than 0.

while (true) {

    for (Blocker blocker : blockers) {
        if (playerRotation == (int) blocker.getRotation()) {
            if(lastBlockerMoved != blocker) {
                lastBlockerMoved = blocker;
                blocker.addAction(Actions.moveTo(blocker.getX(), y, duration, Config.blockerInterpolation));
                blocker.addAction(GapSizeAction.getGapSizeAction(gap, duration));

                playerRotation--; //-- is going right, ++ would be going left
                if (playerRotation > 360)
                    playerRotation -= 360;

                if (playerRotation <= 0)
                    playerRotation += 360;

            }
        }
    }
    // If game over -> break;
}

I am not sure of what condition you need to end this game, but you will need to add this within your while loop to break out.


More efficient conversion of tab-formatted nested data structure to json I have data that looks like the "Input" below and need to convert it into JSON.  My solution works by parsing the text to find a level for each data point.  Then I use a recursive structure to build a JSON tree (or maybe its not JSON, but its much more useful than the original format).

First, I transform the input in the following way.

Input:

person:
    address:
        street1: 123 Bar St
        street2:
        city: Madison
        state: WI
        zip: 55555
    web:
        email: boo@baz.com

First-step output:

[{'name':'person','value':'','level':0},
 {'name':'address','value':'','level':1},
 {'name':'street1','value':'123 Bar St','level':2},
 {'name':'street2','value':'','level':2},
 {'name':'city','value':'Madison','level':2},
 {'name':'state','value':'WI','level':2},
 {'name':'zip','value':55555,'level':2},
 {'name':'web','value':'','level':1},
 {'name':'email','value':'boo@baz.com','level':2}]

This is easy to accomplish with split(':') and by counting the number of leading tabs:

def tab_level(astr):
    """Count number of leading tabs in a string
    """
    return len(astr)- len(astr.lstrip('\t'))

Then I feed the first-step output into the following function:

def ttree_to_json(ttree,level=0):
    result = {}
    for i in range(0,len(ttree)):
        cn = ttree[i]
        try:
            nn  = ttree[i+1]
        except:
            nn = {'level':-1}

        # Edge cases
        if cn['level']>level:
            continue
        if cn['level']<level:
            return result

        # Recursion
        if nn['level']==level:
            dict_insert_or_append(result,cn['name'],cn['value'])
        elif nn['level']>level:
            rr = ttree_to_json(ttree[i+1:], level=nn['level'])
            dict_insert_or_append(result,cn['name'],rr)
        else:
            dict_insert_or_append(result,cn['name'],cn['value'])
            return result
    return result

where:

def dict_insert_or_append(adict,key,val):
    """Insert a value in dict at key if one does not exist
    Otherwise, convert value to list and append
    """
    if key in adict:
        if type(adict[key]) != list:
            adict[key] = [adict[key]]
        adict[key].append(val)
    else:
        adict[key] = val

The approach is redundant and therefore inefficient.  I also wonder whether the solution is robust (for example, I had to modify the code to accommodate repeated tags).  Think of the Input above as a formatting for SGML.  Any suggestions for improvement would be greatly appreciated!

I have not programmed in python but you should be able to do this in one shot. In pseudo-code it should be something like so:

function parseJsonInput (file)
   var parsedJson= {};
   var parentStack = [parsedJson]
   for each line in file
     var data = parseLine(line) //return key,value,level. null if not avail for each field

     //leaf item process it by adding it to its current parent
     if data.value is not null
        var currentParent = parentStack.getLastElement()
        currentParent[data.key] = data.value
        var nextLineLevel = parseLine( peekNextLine() ).level; //peek next line level
        if nextLineLevel = data.level - 1
          parentStack.pop() //done processing child, about to go back to parent
     else
        //group node, push it as the new parent and keep on processing.
        //created more variable than needed for clarity
        var currentParent = parentStack.getLastElement()
        currentParent[data.key] = {}
        var newParent = currentParent[data.key]
        parentStack.push( newParent )
   endfor

    return parsedJson;

end function

I haven't tried that but that should work give or take few bugs. But the basic idea as I mentioned in the comment is to transverse the file iteratively as a tree structure using iterative post-tree-transversal method. I'm not sure if the 'peekNextLine' is available to you, if not then you would need another variable to keep track of the last level processed and strategically insert that logic there -- I started doing this, but figured it might be confusing at first. Let me know if there is some issues. I can help. If I have time I can even write a quick javascript version for you to learn from.


Finding ways to achieve a target sum using recursion I need your review and suggestions how to make this code better. I'm just learning recursion and sure that this solution is not ideal.

Question:

Write a function that given an input array of integers with a varying length input integers array, and the desired target_sum, returns the number of combinations, of any length, that add up to that target_sum.

EXAMPLE 1:

total_combinations = calculate_combinations([5, 5, 15, 10], target_sum=15)

ANSWER 1:

Should return 3, as there are 3 combinations of numbers from the input array that add up to 15, namely:

15 5+10 5+10 10

solution:

private static int mainfunction(int[] input, int target)
{
    int result = 0;
    for (int i = 0; i <= input.Length - 2; i++)
    {
        int temp = mainfunction_slave(input, i, target, 1);
        result = result + temp;
    }
    return result;
}
private static int mainfunction_slave(int[] input, int index, int target, int check)
{
    if ((index  + check) > (input.Length - 1))
        return 0;
    if ((input[index] + input[index + check]) == target || (input[index] == target))
        return mainfunction_slave(input, index, target, check + 1) + 1;
    return mainfunction_slave(input, index, target, check + 1);
}

The task needs to be done via recursion, but if you have any other better solution, please let me know.

A couple of quick notes:

Methods in C# should be named PascalCase.
mainfunction isn't a great name. It should be named by what it does
mainfunction_slave is also a bad name, you shouldn't use underscores in method names in C#

You need to plan your recursion:

What's the base case?
What can you make smaller/approach the base case as you go?

The base case is fairly trivial:

If the array has 1 element, return whether that element matches the target.

static int CalculateCombinations(int[] numbers, int target)
{
    // trivial solution:
    if (numbers.Length == 0)
    {
        return 0;
    }

    // base case:
    if (numbers.Length == 1)
    {
        return numbers[0] == target ? 1 : 0;
    }
}

Cool, we could now (or even before this) write a test or two:

[TestMethod]
public void CaclculateCombinationsWithSingleElement()
{
    var numbers = new[] { 1 };
    var target = 1;

    var result = YourClass.CalculateCombinations(numbers, target);

     Assert.AreEqual(1, result);
}

(I don't use MSTest myself so not sure if the above is 100% correct).

Now you need to think about how to split the problem up. Recursion on a list is almost always making the list smaller at each step.

The number of combinations is:

var count = 0;
for (var i = 0; i < numbers.Length; i++)
{
    // are we at a solution ?
    if (numbers[i] == target)
    {
        count++;
    }

    // find solutions that sum to the current target - the current number
    // on remainder of the array
    var subArray = numbers.Skip(i + 1).ToArray();
    count += CalculateCombinations(subArray, target - numbers[i]);
 }
 return count;

If you only consider positive integers, it looks like target is getting smaller too but actually only the numbers array is getting 'smaller' as negative numbers or 0s don't make target closer to 0.

That makes the whole solution something like:

static int CalculateCombinations(int[] numbers, int target)
{
    if (numbers.Length == 0)
    {
        return 0;
    }
    if (numbers.Length == 1)
    {
        return numbers[0] == target ? 1 : 0;
    }

    var count = 0;
    for (var i = 0; i < numbers.Length; i++)
    {
        if (numbers[i] == target)
        {
            count++;
        }
        var subArray = numbers.Skip(i + 1).ToArray();
        count += CalculateCombinations(subArray, target - numbers[i]);
    }
    return count;
}

Update:

On reflection, the base case can simply be an empty array. We don't need to worry about the length = 1 case because it would have been counted in the for loop anyway.

That means the code can be simplified to:

static int CalculateCombinations(int[] numbers, int target)
{
        if (numbers.Length == 0)
        {
            return 0;
        }

        var count = 0;
        for (var i = 0; i < numbers.Length; i++)
        {
            if (numbers[i] == target)
            {
                count++;
            }
            var subArray = numbers.Skip(i + 1).ToArray();
            count += CalculateCombinations(subArray, target - numbers[i]);
        }
        return count;
    }

I'd recommend The Little Schemer as a great introduction to recursion. One of the commandments is:

  Simplify only after the function is correct.

So I don't feel too bad about overcomplicating it the first time :)


is_numeric_array() is missing I found that in PHP (or I probably can't find it) a proper is_numeric_array($array) function is missing. So I created one. The problem is that I don't think it's great and I don't know how to improve it.

Any suggestion?

My first function

function is_numeric_array($array)
{
    $i = 0;
    foreach ($array as $a => $b) { if (is_int($a)) { ++$i; } }
    if (count($array) === $i) { return true; }
    else { return false; }
}

is_numeric_array(array(0,0,0,0,0)); // true
is_numeric_array(array('str' => 1, 'str2' => 2, 'str3' => 3)); // false

Example

As asked, I provide an example on how this could be any useful.

function is_numeric_array($array)
{
    # Code below
}

function someFunction($array)
{
    if (is_numeric_array($array))
    {
        $query = $array[0];
        $param = $array[1];
        $fetch = $array[2];
    }
    else
    {
        $query = $array['query'];
        $param = $array['param'];
        $fetch = $array['fetch'];
    }

    # Do your sql/pdo stuff here
}

# This use is the same of ...
someFunction(array(
    'PDO SQL STATEMENT',
    array('param1' => 1, 'param2' => 2, 'param3' => 3),
    true
));

# ... this one.
someFunction(array(
    'query' => 'PDO SQL STATEMENT',
    'param' => array('param1' => 1, 'param2' => 2, 'param3' => 3),
    'fetch' => true
));

# To choose one form instead of the other is coder's decision
# Also I know it is useless but I was just wondering why anybody actually looked forward this function

Instead of using a counter to count the keys for which the condition is true, you could just return false as soon as you find a key which is not an int and return true if the loop reaches its end without finding such a key:

foreach ($array as $a => $b) {
    if (!is_int($a)) {
        return false;
    }
}
return true;

This has the benefit that it short-circuits (i.e. it stops iterating once it finds a key that is not an int) and gets rid of the counter.


Error handling Beacon scanning I've recently began trying to put more error handling in to my code so I thought I'd post a use-case to check I'm on the right path. I feel like this is going to make my functions much larger and more indented/nested than before. Is this the right way to go about things or should I be utilising try? instead? Where do people generally put these Error enum definitions?

fileprivate enum LocationError: Error {
    case noAuthorization
    case noBeaconSupport
    case rangingUnavailable
}

class ViewController: UIViewController, CLLocationManagerDelegate {
    @IBOutlet weak var distanceReading: UILabel!
    var locationManager: CLLocationManager!

override func viewDidLoad() {
    super.viewDidLoad()

    locationManager = CLLocationManager()
    locationManager.delegate = self
    locationManager.requestWhenInUseAuthorization()

    view.backgroundColor = UIColor.gray
}

func locationManager(_ manager: CLLocationManager, didChangeAuthorization status: CLAuthorizationStatus) {
    do {
        guard status == .authorizedWhenInUse else {
            throw LocationError.noAuthorization
        }
        guard CLLocationManager.isMonitoringAvailable(for: CLBeaconRegion.self) else {
            throw LocationError.noBeaconSupport
        }
        guard CLLocationManager.isRangingAvailable() else {
            throw LocationError.rangingUnavailable
        }
        startScanning()
    }
    catch LocationError.noAuthorization {
        print("User has not authorized us to use location")
    }
    catch LocationError.noBeaconSupport {
        print("User's device does not support Beacons")
    }
    catch LocationError.rangingUnavailable {
        print("User's device does not support ranging")
    }
    catch {
        fatalError()
    }
}

Throwing Swift errors is a mechanism how a function/method can report a failure to its caller. Your code throws and catches the error within the
same method, and I can see no advantage of using try/catch in that
situation. Your code is equivalent to

func locationManager(_ manager: CLLocationManager, didChangeAuthorization status: CLAuthorizationStatus) {
    guard status == .authorizedWhenInUse else {
        print("User has not authorized us to use location")
        return
    }
    guard CLLocationManager.isMonitoringAvailable(for: CLBeaconRegion.self) else {
        print("User has not authorized us to use location")
        return
    }
    guard CLLocationManager.isRangingAvailable() else {
        print("User's device does not support ranging")
        return
    }
    startScanning()
}

which is

shorter and easy to understand,
makes the enum LocationError obsolete (which is fileprivate,
and therefore apparently not used anywhere else),
makes the catch-all with fatalError() obsolete.

I would even go a step further: guard is useful in connection with
optional binding (to avoid the "optional binding pyramid of doom").
In your case, the same can be achieved with a simple if/else if/.../else statement:

func locationManager(_ manager: CLLocationManager, didChangeAuthorization status: CLAuthorizationStatus) {
    if status != .authorizedWhenInUse {
        print("User has not authorized us to use location")
    } else if !CLLocationManager.isMonitoringAvailable(for: CLBeaconRegion.self) {
        print("User has not authorized us to use location")
    } else if !CLLocationManager.isRangingAvailable() {
        print("User's device does not support ranging")
    } else {
        startScanning()
    }
}


`read` re-implemented for Int's I have written a safe read function for haskell (for education!). I think this is fairly idiomatic haskell code, but I'd like to know for sure. If you see anything else that could be improved, please let me know in your answer.

module ReadNum (readNum) where

main = print $ readNum "87k7"

readNum :: String -> Maybe Int
readNum xs = readNum' $ reverse xs

readNum' :: String -> Maybe Int
readNum' (x:xs) = (\y z -> y + 10 * z) <$> readChr x <*> readNum' xs
readNum' [] = Just 0

chrToNum :: [(Char, Int)]
chrToNum = [('0', 0), ('1', 1),
            ('2', 2), ('3', 3),
            ('4', 4), ('5', 5),
            ('6', 6), ('7', 7),
            ('8', 8), ('9', 9)]

readChr :: Char -> Maybe Int
readChr c = lookup c chrToNum

The change I'd make, respecting your overall design, is to refactor chrToNum as this:

chrToNum :: [(Char, Int)]
chrToNum = zip "0123456789" [0..]

Apart from that, if we are willing to slightly alter the design, I would separate the string validation from the computation of the Int represented. The string validation can be done using mapM :: (a -> m b) -> [a] -> m [b] (this way we avoid explicitly writing the recursion).

validateString :: String -> Maybe [Int]
validateString = mapM readChr

Then, you can define a function computing the Int represented by a list of digits (I use foldl' which requires you to add import Data.List but you can equally well use reverse + foldr as you originally did)

represents :: [Int] -> Int
represents = foldl' (\ ih z -> 10 * ih + z) 0

Finally, putting it all together we get:

readNum :: String -> Maybe Int
readNum = fmap represents . validateString


Optimize program to test for divisibility of numbers 3.0 This is a follow up on my previous question.

JS1's answer suggested that I should use a precomputed table containing all permutations of the lowest valid number for each number between 1 and MAX. It was well described how to implement the precomputed table but somewhere I seem to have miserably failed since the code runs much slower than my previous versions of the code.

I don't believe there's anything wrong with my lookup table but if that's for any use here is the code I wrote to obtain all permutations for numbers between 1 and MAX.

Something with my code seems off because for larger calculations (the example found in my previous question) the code runs terribly slow.

#include <stdio.h>

int lookup[] = {
#include "table.h"
};

#define LENGTH(x) (sizeof(x) / sizeof((x)[0]))
#define MAX 1000000

int main(void) {
  int n[100];
  int x, i, j, k, l, m, N;

  scanf("%d", &N);
  for(i=0;i<N;i++) scanf("%d", &n[i]);
  for(x=1;x<MAX;x++) {
    j=x-1;
    i=-x;
    while((unsigned int)(j) < LENGTH(lookup) && lookup[j] != i) ++j;
    for(l=0;l<N;l++) {
      m = j;
      while((k = lookup[++m]) > 0) {
        if(k%n[l]==0)break;
      }
      if(k<0) goto outer;
    }
    break;
    outer:
    continue;
  }
  fprintf(stdout, "%d\n", x);
}

Perhaps the access time on the array is affecting the speed?

Table generator

Your table generator has a bug, which caused it to crash on my machine.  This line:

  if(*len > buflen) {

should be:

  if(*len >= buflen) {

Second, your table generator is outputting entries for permutations that aren't required.  For example, since 12 and 21 are permutations of each other, you only need to output "-12 21", and later on, there shouldn't be a "-21" entry because you've already checked for 21.  But currently, your table includes both, so your program will check entries that it doesn't need to check.

Incorrect table searching

There are a couple of things wrong with your program.  The biggest problem is that you are starting over at index 1 every time through the loop.  You really don't have to do that because to get to the next number, you just need to skip all the negative numbers.  In other words:

   for(x=1;x<MAX;x++) {
     j=x-1;
     i=-x;

should be:

    j = 0;
    for(x=1;x<MAX;x++) {
      i=-x;

Just making that change with your current table reduces the program time back down to "normal" levels, although it still isn't correct.

A second problem is that you are skipping lookup[j] when doing the dividing loop.  This is because your loop uses a preincrement, most likely because you are trying to avoid the first number of the sequence, which is negative.

  m = j;
  while((k = lookup[++m]) > 0) {

What you can do is change the table's negative entry to positive.  Then you can use a postincrement:

lookup[j] = -lookup[j];
for(l=0;l<N;l++) {
  m = j;
  while((k = lookup[m++]) > 0) {
    if(k%n[l]==0)break;
  }
  if(k<0) goto outer;
}

Searching the optimized table

Once you start using a table that doesn't include redundant entries, your search will need to change.  Your current search looks for the next number.  But since not every number will exist, all you need to do is search for a non-negative number.  Also, I appended a -1000000 entry at the end of my table to make sure I didn't run past the end of the table.

Your current search:

 while((unsigned int)(j) < LENGTH(lookup) && lookup[j] != i) ++j;

would turn into:

while (lookup[j] >= 0)
    ++j;

Putting it all together

This is what your code would look like with all the changes.  This code assumes that the table you generate has a -1000000 entry at the end and contains no redundant entries:

#include <stdio.h>

int lookup[] = {
#include "table.h"
};

#define MAX 1000000

int main(void) {
  int n[100];
  int i, j, k, m, N;

  scanf("%d", &N);
  for(i=0;i<N;i++) scanf("%d", &n[i]);
  for(i=0;i<MAX;i++) {
    while(lookup[i] >= 0)
        ++i;
    lookup[i] = -lookup[i];
    for(j=0;j<N;j++) {
      m = i;
      while((k = lookup[m++]) > 0) {
        if(k%n[j]==0)break;
      }
      if(k<0) goto outer;
    }
    break;
    outer:
    continue;
  }
  fprintf(stdout, "%d\n", lookup[i]);
}

You can find my own version of the table generator and solver program here on Github.


Mapping a user's availabilities So we're starting to use Typescript for a new project at work. Part of the application allows a user to retrieve their future availabilities (present/absent/possibly present), for planning purposes.

This is the get function of the service right now.

getMyAvailabilities(startDate: Moment, endDate : Moment): Observable<Map<Moment, Availability>>
{
    let params = new URLSearchParams();
    params.set('startDate', startDate.format(this.formatDateString));
    params.set('endDate', endDate.format(this.formatDateString));
    return this.http.get(this.resourceUrl, {search: params})
        .map((res: Response) =>
             {
                 let serializedResult = new Map<Moment, Availability>();
                 let result = res.json();
                 for (let key of Object.keys(result))
                 {
                     let availability = result[key];
                     serializedResult.set(moment(key),
                                          availability);
                 }
                 return serializedResult;
             });
}

You provide a startDate, and an endDate to the API, and the API will give you a map of availabilities with the date as a key.

I wrote a functional variant because it seemed suitable to functional code. We're using a lot of functional-style code in our project, because it's very suited for transforming lists of objects.

getMyAvailabilities(startDate: Moment, endDate: Moment): Observable<Map<Moment, Availability>>
{
    let params = new URLSearchParams();
    params.set('startDate', startDate.format(this.formatDateString));
    params.set('endDate', endDate.format(this.formatDateString));
    return this.http.get(this.resourceUrl, { search: params }).map((res: Response) => {
        let result = res.json();
        return Object.keys(result)
            .reduce((map: Map<Moment, Availability>, key: string) => {
                map.set(moment(key), result[key]);
                return map;
            }, new Map<Moment, Availability>());
    });
}

So, the functional variant is shorter. It's not likely that this function will need to map a large amount of objects - right now, we're estimating about 10-60 objects.

Most of us have a Java background - we're used to programming in Java, we're new or new-ish to the functional style of programming, so when presented with a procedural-style snippet of code and a functional-style snippet of code which do the same, perform the same and are of the same length, we tend to pick the procedural-style code.

But for this new project, making heavy use of procedural code would clash with the functional style used throughout the rest of the project.

Thus, the dilemma:

If we're not sure whether a piece of code is better as functional or procedural, should we go with the procedural code, which is easier to understand and probably more performant, even although we don't need that performance? Or should we go with the functional code, which is harder to understand, but follows the style of the rest of the project and is shorter in LoC?

And for the bonus question:

Is that a proper use of .reduce? Usually reduce gives a single value back. In this case, it returns... a collection. Is there another way by which we can go from Object (which has string keys and Availability values) to Map<Moment, Availability>?

Another variant of the code

I took your second (more functional) example and polished it a bit.

*) The const keyword is used instead of the let in two places. This prevents accidental/undesired re-assignment of a value to a local variable.

**) the type of the accumulatorMap and key can be inferred.

***) The .reduce() is simplified, since map.set(key, value) already returns you a map object.

getMyAvailabilities(startDate: Moment, endDate: Moment): Observable<Map<Moment, Availability>>
{
  const params = new URLSearchParams();                              // *
  params.set('startDate', startDate.format(this.formatDateString));
  params.set('endDate', endDate.format(this.formatDateString));

  return this.http
    .get(this.resourceUrl, { search: params })
    .map((response: Response) => {
      const result = response.json();                                // *
      return Object.keys(result)
        .reduce(
          (accumulatorMap, key) =>                                   // **
            accumulatorMap.set(moment(key), result[key]),            // ***
          new Map<Moment, Availability>()
        );
    });
}

Functional vs imperative — hopefully not off topic

Disclaimer

I don't think that Codereview.StackExchange is the right place to ask questions of that sort. The answers may get extremely subjective (and result in a "holy war"). The issue is that there's no universally objective criteria which could be used to justify that functional code is by definition better than imperative ...or vice versa.

I prefer functional...

Still, I chose the functional code snippet you provided and improved it, rather than the imperative. Here's the reason why, but please take it as an opinion.

In my experience, functional code is less buggy, so to speak. Now, I do not have any data to back this statement, and my guess is that the scientific studies on that subject are contradictory. Nevertheless, this is what I feel about it.

Reasoning behind "functional is better"

Here are a very few functional programming aspects and their possible effects:

Working with Maybe/Option [monad] makes the code more generalized, gets rid of explicit branching —> Simpler code. —> Defect likelihood reduces.
Using const instead of let or var prevents accidental object reassignment. No if-guards are needed. —> Defect likelihood reduces.
Pure functions do not modify the state, nor alter the input parameters, and always return same results for same inputs. —> Simpler code. —> Defect likelihood reduces.
...
I think you get my point but please do not trust me! Search online for more in-depth articles or blog posts. :)

Psychology matters

However, as you said, it takes time to adjust one's mindset, way of thinking, or habits — call it whatever you want — to harness functional programming aspects.

Remember how you touched a declarative (e.g. SQL), logic (e.g. Prolog), functional (e.g. Haskell, Lisp, Scala) or any other fundamental paradigm language for the first time. I bet it was not easy to grasp. For the first several weeks or months everyone in this position will write a lot of imperfect code and it's totally fine (as long as he/she is willing to revisit and fix it).

During this period the code may be more buggy, but this effect is temporary and diminishes with experience and learning time investments. It is important from the psychological standpoint to not make any conclusions at this point about whether the paradigm being adopted is good or bad. For the same reasons, it's extremely important to not judge a dev nor his code too strictly.

Anesthesia?

If you agree that the benefits are sweet, you want to push for functional code when possible. Still, the transition process may be painful. What do you do?

Well, here're the good news. The Divide and Conquer principle can be successfully applied. In fact, this is how I learn functional programming myself. You don't need to learn all the aspects at once (just like the SOLID principles, or design patterns).

Add one tool at a time to your toolkit and start harvesting the fruits.
Polish your skills over time and they become your second nature.
Experiment with what you've learnt.
Keep investing into the tools (in other words, make sure point 1. is repeated).
Retrospect and notice how the practices/aspects can be efficiently combined together. Often the case it happens intuitively without intentional search for it.
Be patient...
PROFIT!

P.S.

Many programming languages that have functional traits don't push a developer very hard towards writing functional code. This is especially true for the languages that acquired these traits as a result of evolution (like C# and latest versions of Java). Developers can still write imperative code even in Scala, the language does not prevent that.

Obviously, there's no silver bullet in programming. There will always be bugs out there, no matter what king of programming language we're talking about. It's the human beings who write the code, and we are not perfect in it. Realistic expectations may help a lot to achieve satisfaction instead of disappointment.


Swift Prim's algorithm Lots of Prim's algorithm implementations seem long, and rely on a priority queue implementation.

If I use an adjacency matrix I can then calculate the next item to take using functional programming in Swift.

func prims (_ matrix : [[Int]]) {
    var selected = 0
    var selectedSoFar = Set<Int>()
    selectedSoFar.insert( (matrix[selected].enumerated().min{ $0.element < $1.element }?.offset)! )

    while selectedSoFar.count < matrix.count {
        var minValue = Int.max
        var minIndex = selected
        var initialRow = 0
        for row in selectedSoFar {
            let candidateMin = matrix[row].enumerated().filter{$0.element > 0 && !selectedSoFar.contains($0.offset) }.min{ $0.element < $1.element }
            if (minValue > candidateMin?.element ?? Int.max ) {
                minValue = candidateMin?.element ?? Int.max
                minIndex = (candidateMin?.offset) ?? 0
                initialRow = row
            }
        }
        print ("edge value \(minValue) with \(initialRow) to \(minIndex)")
        selectedSoFar.insert(minIndex)
        selected = (minValue)
    }
}

let input = [[0,9,75,0,0],[9,0,95,19,42],[75,95,0,51,66],[0,19,51,0,31],[0,42,66,31,0]]
prims(input)

Essentially is there anything "wrong" with this implementation? This is not homework, my code runs correctly for the included output, I've stepped through the output with pen and paper, and I appreciate this is faster if I use an adjacency list. However the Ray Wenderlich implementation of Prims uses 7 files and >200 lines of code. Am I missing something?

Am I missing something?

Each expansion of the tree visits every vertex, either as a member of selectedSoFar or as a candidate in matrix[row].  That makes the runtime \$O(n^2)\$ in the number of vertices.

An algorithm based on ordered data will tend toward \$O(n*log(n))\$.  With a million vertices, that's 50 thousand times faster—the difference between an hour and six years.

See also Wikipedia on Prim's; there's a section on time complexity.


Comparing dates (without the time part) in Java In my current project, I have to deal a lot with dates, especially checking if the date entered by user is today, or max one year from today etc. As I find the datetime API of Java7 (which I regrettably still have to use) pretty confusing, in order to keep my code clean and readable, I wrote the following method:

public static int compareDates(Date date1, Date date2) {
    SimpleDateFormat math = new SimpleDateFormat("yyyyMMdd"); // I'm working with servlets, so keeping it thread-safe
    Long date1asLong = new Long(math.format(date1));
    Long date2asLong = new Long(math.format(date2));
    return date1asLong.compareTo(date2asLong);
}

// usage, e.g. to find if the date entered by user is greater than today
if (MyDateUtils.compareDates(dateFromUser, new Date()) > 0) {
    // do the work
} else {
    // scream
}

Anyway, I am not sure whether this approach has any serious drawbacks, so I am throwing it here for review. Looking forward to your comments.

It's an interesting, simple approach.

But you don't need new Long objects. Better use primitive types instead.

In fact, as @njzk2 pointed out in a comment,
you don't need long at all, as dates in yyyyMMdd format are alphabetically sorted, you can use simple String comparison.

It's good you mention in comments that you create a SimpleDateFormat instance inside the method for thread safety.
Another common solution in such situations is to use a ThreadLocal.
You can wrap a SimpleDateFormat instance in a ThreadLocal so that all threads will have their own copy, eliminating thread safety issues.

Last but not least, "math" is a poor name for a date formatter object.

Putting my suggestions together:

private static final ThreadLocal<DateFormat> dateFormatThreadLocal = new ThreadLocal<DateFormat>() {
    @Override
    protected DateFormat initialValue() {
        return new SimpleDateFormat("yyyyMMdd");
    }
};

public static int compareDates(Date date1, Date date2) {
    DateFormat dateFormat = dateFormatThreadLocal.get();
    return dateFormat.format(date1).compareTo(dateFormat.format(date2));
}


Getting a min birth year and max death year from a list of years A very simple problem here (I'm learning Python) - given a list of tuples which represent (year of birth, year of death) for a group of people, I want to find the minimum "year of birth" and the maximum "year of death". Here's how I did it:

def testmax():
    years = [(1804, 1884), (1885,1997), (1902,1975), (1900,1989)] # sample data

    endyr = max([deathyr for (birthyr,deathyr) in years])
    startyr = min([birthyr for (birthyr,deathyr) in years])
    print(startyr, endyr) # correctly returns 1804 1997

As you can see, I use list comprehensions to go through the years list twice. Time complexity O(2N) = O(N).

I can improve this with a simple function that is longer but does it through one iteration of the years list — I simply track (min, max) as my return variables that I keep checking on every iteration to get the min and max years that I need, in one iteration. Time complexity = O(N). What I don't like here is writing longer code (perhaps in a test/interview where time is limited).

My question: is there a better/cleaner/more Pythonic way you would do this? Would a lambda function work here, if so how? Even if you still do it in 2 iterations, I'd appreciate it if you could demo/explain any variations of the code (perhaps using lambda) that you would have used.

What you really want to do is transpose the separate tuples into 2 tuples - one containing all the birth dates, and the other death dates.

Fortunately, python's zip function can do this easily for you, using the *x syntax:

years = [(1804, 1884), (1885,1997), (1902,1975), (1900,1989)]
births, deaths = zip(*years)
# births = (1804, 1885, 1902, 1900)
# deaths = (1884, 1997, 1975, 1989)
print(min(births), max(deaths))
# 1804 1997


Function that adds two integer strings I wrote this method that adds two integers in the form of strings together. This way I can add large integers without overflowing.

How efficient is the code that I wrote? I am by no means a great coder, but I would consider myself somewhat decent. Is there anyway that would make it better?

I am a self taught coder and have only been doing it for about a year, so please be easy on me.

static string addString(string num1, string num2) {
        string retVal = null, cNum1 = null, cNum2 = null;
        int bf1 = 0, bf2 = 0;
        if (num1.Length > num2.Length) {
            for (int i = 0; i < (num1.Length - num2.Length); i++) {
                cNum2 += "0";
            }
        } else if (num1.Length < num2.Length) {
            for (int i = 0; i < (num2.Length - num1.Length); i++) {
                cNum1 += "0";
            }
        }
        cNum1 += num1;
        cNum2 += num2;
        char[] cAN1 = cNum1.ToCharArray();
        Array.Reverse(cAN1);
        cNum1 = new string(cAN1);
        char[] cAN2 = cNum2.ToCharArray();
        Array.Reverse(cAN2);
        cNum2 = new string(cAN2);
        for (int i = 0; i < cNum1.Length; i++) {
            bf2 = bf1 + int.Parse(cNum1.Substring(i, 1)) + int.Parse(cNum2.Substring(i, 1));
            if (bf2 > 9) {
                retVal += bf2.ToString().Substring(bf2.ToString().Length - 1, 1);
                bf1 = int.Parse(bf2.ToString().Substring(0, bf2.ToString().Length - 1));
            } else {
                retVal += bf2.ToString();
                bf1 = 0;
            }
        }
        if (bf1 != 0) {
            retVal += bf1.ToString();
        }
        char[] ret1 = retVal.ToCharArray();
        Array.Reverse(ret1);
        retVal = new string(ret1);
        return retVal;
    }

Some style comments

Add vertical spaces to group code which logically belongs together.
Don't declare multiple variables on the same line.
Name your variables as meaningfully as possible. Names like bf1 won't tell Bob the maintainer what it is about, making it harder to maintain the code.
methods should be named using PascalCase casing. See: C# naming guidelines
Usually a C# developer would expect one to use the Allman indention style; i.e. place the opening brace on the next line.
Always add a scope specifier (access modifier) to methods and variables.

This method is violating the Single responsibility principle. It is just doing too much like normalizing the given input, adding the numeric values of the chars and constructing the result.

Let us take a look at what you are doing to normalize the string

    string cNum1 = null;
    string cNum2 = null;
    if (num1.Length > num2.Length) {
        for (int i = 0; i < (num1.Length - num2.Length); i++) {
            cNum2 += "0";
        }
    } else if (num1.Length < num2.Length) {
        for (int i = 0; i < (num2.Length - num1.Length); i++) {
            cNum1 += "0";
        }
    }
    cNum1 += num1;
    cNum2 += num2;

Each time you call cNum1 += "0" or cNum2 += "0" you are creating a new string. By using the PadLeft() method together with Math.Max() and extracting the the normalization to a separate method your code will look cleaner.

private static string NormalizeValue(string value, int maxLength)
{
    return value.PadLeft(maxLength, '0');
}

This has also the advantage that you won't need cNum1 and cNum2 anymore.

static string addString(string num1, string num2) {

    int maxLength = Math.Max(num1.Length, num2.Length);
    num1 = NormalizeValue(num1, maxLength);
    num2 = NormalizeValue(num2, maxLength);

    .....

}

If you start the for loop at maxLength - 1 and decrement the index you won't need the reversing of the char arrays anymore.

Using a StringBuilder to add the results of the single additions will result in fewer created strings.

If you are sure that the passed in strings only contain digits you can use a neat little trick. You can subtract a char '0' from the current char like e.g

Assume first = '2' and second = '4' are the chars currently be processed.

int result = first - '0' + second - '0';

this boils down to

int result = '2' - '0' + '4' - '0' ;

and is subtracting the ASCII values of the chars

int result = 50 - 48 - 52 - 48;

so there is no need to call int.Parse() anymore. By extracting the two uses of '0' to a const this will be beautified.

const int doubleZeroValue = 96;
int result = first + second - doubleZeroValue;

This will lead to

private static string NormalizeValue(string value, int maxLength)
{
    return value.PadLeft(maxLength, '0');
}

private static string AddString(string firstNumber, string secondNumber)
{
    const int doubleZeroValue = 96;
    int maxLength = Math.Max(firstNumber.Length, secondNumber.Length);
    firstNumber = NormalizeValue(firstNumber, maxLength);
    secondNumber = NormalizeValue(secondNumber, maxLength);

    StringBuilder sb = new StringBuilder(maxLength + 1);
    int result = 0;
    for (int i = maxLength - 1; i >= 0; i--)
    {
        result += firstNumber[i] + secondNumber[i] - doubleZeroValue;
        if (result < 10)
        {
            sb.Append(result);
            result = 0;
            continue;
        }
        sb.Append(result - 10);
        result /= 10;

    }

    if (result > 0)
    {
        sb.Append(result);
    }

    char[] resultDigits = sb.ToString().ToCharArray();
    Array.Reverse(resultDigits);

    return new string(resultDigits);

}

It still has some magic numbers in it like

sb.Append(result - 10);
result /= 10;

which could/should be extracted to meaningful constants but I couldn't come up with the names lacking enough english.