Concatenating prefix of one array with the reversed suffix of another array Given two arrays and two indices, I need to concatenate the prefix of the first array and the reversed prefix of the second array.

For example:

// Input
arr1 = [1,2,3,4,5,6,7,8,9];
arr2 = [10,11,12,13,14,15];
ind1 = 6;
ind2 = 3;

// Output
arr = [1,2,3,4,5,6,12,11,10];

I have implemented two different functions for this purpose:

function func1(arr1,arr2,ind1,ind2)
{
    let arr = [];
    for (let n = 0; n <= ind1; n++)
        arr.push(arr1[n]);
    for (let n = ind2; n >= 0; n--)
        arr.push(arr2[n]);
    return arr;
}

function func2(arr1,arr2,ind1,ind2)
{
    return arr1.slice(0, ind1 + 1).concat(arr2.slice(0, ind2 + 1).reverse());
}

And now I would like determine which one of them is better, using this performance analysis:

function measure(func,arr1,arr2,ind1,ind2)
{
    let t0 = performance.now();
    arr = func(arr1,arr2,ind1,ind2);
    let t1 = performance.now();
    return t1 - t0;
}

results = '';

for (let len = 10; len <= 120; len += 10)
{
    let func1Count = 0;
    let func2Count = 0;
    for (let i=0; i<1000000; i++)
    {
        let arr1 = new Array(len);
        let arr2 = new Array(len);
        let ind1 = Math.floor(Math.random()*len);
        let ind2 = Math.floor(Math.random()*len);
        func1Count += measure(func1,arr1,arr2,ind1,ind2);
        func2Count += measure(func2,arr1,arr2,ind1,ind2);
    }
    if (func1Count < func2Count)
        results += 'len='+len+': func1 is better ('+func1Count+' vs '+func2Count+')\n';
    else
        results += 'len='+len+': func2 is better ('+func1Count+' vs '+func2Count+')\n';
}

alert(results);

The test above indicates that the first function is better if the array length is (approximately) 80 or less, and that the second function is better otherwise.

I would like to ensure the correctness of my performance analysis.

In addition, I should note that this code will eventually run on server side (i.e., NodeJS), but I am testing it on client side (i.e., a browser). So if this method is fundamentally wrong (i.e., testing a server-side code over a client-side platform), then I would be happy to be notified.

The only note I can make is that in func1 and func2 you have an "API bug": the indices are "included" in the array. Consider this example:

[1, 2, 3, 4].slice(1, 3)

will produce [2, 3], so the point is that 3 in slice(1, 3) is not included. This does not happen only in Javascript, but also in other languages as well (I know Java does it that way). So the only advice would be to write your functions as

function func1(arr1,arr2,ind1,ind2)
{
    let arr = [];
    for (let n = 0; n < ind1; n++)
        arr.push(arr1[n]);
    for (let n = ind2 - 1; n >= 0; n--)
        arr.push(arr2[n]);
    return arr;
}

function func2(arr1,arr2,ind1,ind2)
{
    return arr1.slice(0, ind1).concat(arr2.slice(0, ind2).reverse());
}

Also, it is good to have a single space after each comma , in the function parameter list.


Pass Async function to new Promise constructor I'm having a hard time wrapping my head around whether or not this works or how I can even test it but....

I want to use Async/Await inside a Promise, which means, when I create a new Promise, I have to pass it an async function... something like this:

const getData = function(){
  return new Promise(async function(resolve, reject){
    try {
      const results = await axios.get(some_data);
      resolve(results);
    }
    catch(er) { reject(er); }
  })
}

I've tried to test it myself, but I can't tell if it's working correctly or not. Anyone try this before?

There's simply no reason to wrap your existing promise with another promise and no reason to use await or async here either:

const getData = function(){
  return axios.get(some_data);
}

Wrapping an existing promise with a manually created promise is a promise anti-pattern and should not be done.  Instead, you should just return the promise you already have.

If you're going to use more than one async operation and thus find you want to use await rather than chain your promises with .then(), then you can do that like this:

const getData = async function(){
  let val = await axios.get(some_data);
  let otherVal = await someOtherFunction();
  return something;
}

Note also that if you want a rejected promise to just get returned as a rejection, you don't have to catch it locally in an async function.  If you one of the promises you are using await on rejects, that will just reject the promise that your async function returns with that error.

If you wanted to catch the error locally and do something about it, then you would use try/catch:

const getData = async function(){
  try {
    let val = await axios.get(some_data);
    let otherVal = await someOtherFunction();
    return something;
  } catch(e) {
    // some something with the error and decide what to return
  }
}


Checking whether one string matches another This checks whether one string has all the chars in another string to make up that exact string. Counts matter so for, for example, str1 = 'abcdgbff' str2 ='aaabb' should return false because str1 only has one 'a' and 'b'.

How can I optimize my code further?

function scramble(str1, str2) {
  for (let i of str1) {
    if (str2.indexOf(i) != -1) {
      str2 = str2.slice(0, (str2.indexOf(i))) + str2.slice(str2.indexOf(i) + 1);
    }
  }
  return str2.length === 0;
}

// Example:
console.log(scramble('abcdgbff', 'aaabb')); // false

If your input strings are of length n, your current implementation has a cubic runtime complexity O(n³). This could become an issue for longer strings.

I recommend to split counting character frequencies and comparing those counts into two separate steps with nearly linear complexity O(n):

// Count characters in 'str':
function frequencies(str) {
  let result = {};
  for (let chr of str) {
    result[chr] = (result[chr] || 0) + 1;
  }
  return result;
}

// Check if 'str1' contains all characters of 'str2':
function scramble(str1, str2) {
  let frequencies1 = frequencies(str1);
  let frequencies2 = frequencies(str2);
  return Object.entries(frequencies2).every(([chr2, frequency2]) =>
    frequencies1[chr2] >= frequency2
  );
}

// Examples:
console.log(scramble("abc", "cba"));
console.log(scramble("aa", "aaaa"));
console.log(scramble("123", "234"));


Getting duration between two dates I'm developing on a web application where I have a start and a finish date and time and have to set the duration between it.

There are inputs for dates and times.

Then, I have this function which calculates the duration between the two of them, and they are working properly. The duration, though, needs to be dynamic. So, i made this piece of code, creating a "on" on each of then, so that when it changes, they mount an object with the four datetime values and then call it to the getDuration function.

$(document.body).on('change','.data-inicio > input',function () {
    var tr   = $(this).parent().parent();
    var time = {
        dstart:  dateDbToView($(this).val()),
        hstart:  tr.find('.hora-inicio').find('input').val(),
        dfinish: dateDbToView(tr.find('.data-final').find('input').val()),
        hfinish: tr.find('.hora-final').find('input').val()
    };
    var drc = getDuration(time.dstart, time.hstart, time.dfinish, time.hfinish);
    tr.find('.duracao').find('input').val(drc);
});

$(document.body).on('change','.hora-inicio > input',function () {
    var tr   = $(this).parent().parent();
    var time = {
        dstart:  dateDbToView(tr.find('.data-inicio').find('input').val()),
        hstart:  $(this).val(),
        dfinish: dateDbToView(tr.find('.data-final').find('input').val()),
        hfinish: tr.find('.hora-final').find('input').val()
    };
    var drc = getDuration(time.dstart, time.hstart, time.dfinish, time.hfinish);
    tr.find('.duracao').find('input').val(drc);
});

$(document.body).on('change','.data-final > input',function () {
    var tr   = $(this).parent().parent();
    var time = {
        dstart:  dateDbToView(tr.find('.data-inicio').find('input').val()),
        hstart:  tr.find('.hora-inicio').find('input').val(),
        dfinish: dateDbToView($(this).val()),
        hfinish: tr.find('.hora-final').find('input').val()
    };
    var drc = getDuration(time.dstart, time.hstart, time.dfinish, time.hfinish);
    tr.find('.duracao').find('input').val(drc);
});

$(document.body).on('change','.hora-final > input',function () {
    var tr   = $(this).parent().parent();
    var time = {
        dstart:  dateDbToView(tr.find('.data-inicio').find('input').val()),
        hstart:  tr.find('.hora-inicio').find('input').val(),
        dfinish: dateDbToView(tr.find('.data-final').find('input').val()),
        hfinish: $(this).val()
    };
    var drc = getDuration(time.dstart, time.hstart, time.dfinish, time.hfinish);
    tr.find('.duracao').find('input').val(drc);
});

My question is: this looks pretty confusing for anyone but me, which can be an bad point when it comes about maintenance. There must be a better way of making this work, so, I'm asking for suggestions.

All of the change handlers are basically the same.  You could roll them all into one by using smarter jQuery selectors.

Also, the time object is pointless.  You can just use four separate variables.  I've also moved the dateDbToView() calls to declutter the tr.find(…) lines so that the parallelism is more obvious.

$('.data-inicio, .hora-inicio, .data-final, .hora-final').on('change', 'input', function() {
    var tr = $(this).parent().parent();
    var dStart = tr.find('.data-inicio > input').val();
    var hStart = tr.find('.hora-inicio > input').val();
    var dFinish = tr.find('.data-final > input').val();
    var hFinish = tr.find('.hora-final > input').val();

    tr.find('.duracao > input').val(duration(
        dateDbToView(dStart),  hStart,
        dateDbToView(dFinish), hFinish
    ));
});

I suggest renaming getDuration(…) to duration(…), because it is performing a calculation, rather than retrieving information that already exists.  (For the same reason, we say Math.sin(…) rather than Math.getSin(…)).


getProcID and getProcessHandle functions I made two functions: one to get process id and the other to get a handle with all access to that process using the process id. I'm looking for help with making these easy to use on all types of projects. The only thing I know is maybe letting the person chose the open process properties.

DWORD getProcID(const std::wstring &windowName, std::string &status)
{
    DWORD processID = NULL;
    HWND windowHandle = NULL;

    windowHandle = FindWindowW(NULL, windowName.c_str()); // Gets handle to top window (class name, window name)
    if (windowHandle) {
    GetWindowThreadProcessId(windowHandle, &processID); // returns thread that made window (window, process for window)
            if (processID != 0) {
                    return processID;
            }
            else { // if process id is bad
                status = "Error with process";
                return 0;
            }
    }
    else { // if window name is bad/not there
        status = "Error with window";
        return 0;
    }
    CloseHandle(windowHandle); // close window handle since we dont need it anymore
    return 0;
}

HANDLE &getProcessHandle(const DWORD processID, std::string &status) {
    HANDLE procHandle = NULL;

    if (processID != 0) {
        procHandle = OpenProcess(PROCESS_ALL_ACCESS, FALSE, processID); // returns handle to process (what you want, inherit stuff, process id)
        if (procHandle != INVALID_HANDLE_VALUE || procHandle != NULL) { // if handle returned by openProcess is good
            status = "Game is ready to hack";
        }
        else { // if handle retured by openProcess is bad
            status = "Error with handle";
        }
    }
    else {
        status = "Error with process id";
    }
    return procHandle;
}

I fairly routinely (not so much here as at work, for example) kind of "scold" people for creating classes where they really only need or want a single function.

This, however, is kind of the opposite case: functions where you really want classes. In particular, RAII can provide a real benefit here, and for that you need a class with a dtor.

I'm not excited about the situation with reporting errors by putting them into a string (and hoping that an empty string indicates success). I'd rather follow the model of "succeed or throw".

To keep that simple, I think it can also benefit from defining an assure (or something on that order) that's at least roughly similar to assert, except that it remains intact in release builds, and (perhaps) throws an exception instead of just aborting if a problem is seen. For example:

#define assure(cond, msg) do { if (!(cond)) throw std::runtime_error(msg); } while (0)

With that in hand, I'd probably start with a WindowHandle type something like this:

class WindowHandle {
    HWND handle;
public:
    WindowHandle(std::wstring const &windowName)
        : handle(FindWindowW(nullptr, windowName.c_str()))
    {
        assure(handle != 0, "Window not found");
    }
    operator HWND() { return handle; }
};

Note that (at least as far as I know, or MSDN seems to indicate) you normally don't want to call CloseHandle on the handle returned from FindWindow. If you did want to, you'd do that in the destructor.

Likewise, a process ID would look something like this:

class ProcId {
    WindowHandle handle;
    DWORD id;
public:
    ProcId(std::wstring const &window_name)
        : handle(window_name)
    {
        GetWindowThreadProcessId(handle, &id);
        assure(id != 0, "Error retrieving process ID");
    }
    operator DWORD() { return id; }
};

Finally, we get to the process handle (where we finally do need to use Closehandle):

class procHandle {
    ProcId id;
    HANDLE handle;
public:
    procHandle(std::wstring const &name, DWORD access = PROCESS_ALL_ACCESS)
        : id(name),
        handle(OpenProcess(access, FALSE, id)   )
    {
        // according to MSDN, returns NULL in case of error
        assure(handle != NULL, "Error retrieving process handle");
    }

    ~procHandle() { CloseHandle(handle); }

    operator HANDLE() { return handle; }
};

Note how each of these builds on the previous, so each only does one thing (i.e., we're following the single responsibility principle). Also note how the code to open a process handle becomes completely trivial:

procHandle p(L"SomeWindowName");

If we add in code to print out the error message if something goes wrong, it's still only marginally more complex:

try {
    procHandle p(L"Start");
    // Code to use process handle goes here.
} // handle will be closed automatically when we exit the scope
catch (std::exception const &e) {
    std::cerr << e.what() << "\n";
}

At least to me, this seems both easier to understand, and easier to use (and harder to use incorrectly).


Conditionals for time specific data This is a fun piece. I have this block of code here:

if (minutes >= 60 && hours < 24) {
        return hours + " hours ago";
    } else if (hours >= 24 && days <= 30) {
        return days + " days ago";
    } else if (minutes <= -60 && hours > -24) {
        return -hours + " hours from now";
    } else if (hours <= -24 && days >= -30) {
        return -days + " days from now";
    } else if (minutes >= -60 && minutes < 0) {
        return -minutes + " minutes from now";
    } else if (days > 30 && days < 365) {
        if (days/30 == 1) {
            return "Last month";
        }
        return days/30 + " months ago";
    } else if (days < -30 && days > -365) {
        if (-days/30 == 1) {
            return "Next month";
        }
        return -(days/30) + " months from now";
    } else if (days >= 365) {
        if (days/365 == 1) {
            return "Last year";
        }
        return days/365 + " years ago";
    } else if (days <= -365) {
        if (-days/365 == 1) {
            return "Next year";
        }
        return -(days/365) + " years from now";
    } else {
        return minutes + " minutes ago";
    }

I want to reduce the number of if-else statements. The ways I was thinking of tackling this was:

Store these into a hashmap and then call it \$O(1)\$ time
Use enums
Ternary operators
Switch statements

I don't want to use enums since this is on a mobile device, and enums are heavier on memory. I was thinking that maybe ternary operators are best here. Thoughts?

The negative is intentional. It's with an API that I am working with and they use negatives to imply future events.

Input from API:

This is the following example input from the API:

"time": "2015-03-06 20:00:00 EST"

It is passed to the following code block, and it is expected to return something like:

"45 minutes ago"
"56 seconds ago"
"1 hour from now"
"2 days from now"
"2 months from now"
"Next year"
"Two years from now"
"A month ago"
"28 days ago"
"5 mins from now"
//etc.

I'd just say, WTF is this doing?

You're testing minutes and hours first as if they were the most important things. So with 10 minutes, 1 hours and 1000 days, you output something like "1 hours ago".

Maybe it's impossible as the variables all get computed from a single time interval. But you didn't show us.

I guess it's better for my sanity not to try too hard to figure out what it exactly does. So let me assume that all variable come from a single time interval. Then a condition like

minutes >= 60 && hours < 24

can be expressed like

1 <= hours && hours < 24

This doesn't buy us much as the order is still confusing:

" hours ago"
" days ago"
" minutes from now"
" months ago"

Again, not trying to find a system there.

What about something like

String suffix = minutes < 0 ? "ago" : "from now";

if (Math.abs(days) >= 365) {
    return (Math.abs(days) / 365) + " years " + suffix;
} else if (Math.abs(days) >= 2*30) {
    return (Math.abs(days) / 12) + " months " + suffix;
} else if (Math.abs(days) >= 30) {
    return days < 0 ? "Last month" : "Next month";
} else if (Math.abs(days) >= 1) {
    return Math.abs(days) + " days " + suffix;
} else if (Math.abs(hours) >= 1) {
    return Math.abs(hours) + " hours " + suffix;
} else {
    return Math.abs(minutes) + " minutes " + suffix;
}

I'm not claiming it's correct, but it's short and damn simple, so you find and fix all bugs in a few seconds.

So what I dislike is

The high count of cases, which could be cut in half using the abs idea stolen from Hosch250.
Placing two tests in each if when one is enough. You need no range tests if you do it systematically.
Even with range tests, there should be a clean order.

I always prefer to keep things simple from the very beginning. Otherwise, it can easily happen that you get a cool idea, but can't apply it because of the need to preserve compatibility with some quirks in the original.

Another disadvantage of starting with complicated stuff is that you may miss some simplifications the way I did above. Concerning correctness, all the divisions and comparisons with 356 and 30 are slightly wrong.

Note also that the task is not exactly defined. Given the dates 2000-12-31T23:23:00 and 2001-01-01T00:11:22, multiple answers are correct:

"48 minutes from now"
"nearly one hour from now"
"tomorrow"
"next month"
"next year"

And maybe also "next century" and "next millennium", but both are disputable (and irrelevant most of the time).

There's no clear solution for this and you should specify the behavior precisely and cover it by tests. The most straightforward solution would be to look at the calendar year first, which would give us the answer "next year". Pretty impractical, but every other solution needs some arbitrary choices.

I'd probably start with

String timeDifferenceString(long currentTimeMillis, long otherMillis) {
    final boolean isPast = otherMillis < currentTimeMillis;

    final Calendar first = Calendar.getInstance();
    first.setTimeInMillis(Math.min(currentTimeMillis, otherMillis));

    final Calendar second = Calendar.getInstance();
    second.setTimeInMillis(Math.max(currentTimeMillis, otherMillis));

    return timeDifferenceString(first, second, isPast);
}

so that no Math.abs is needed anymore. Obviously, Joda-Time or the corresponding JDK8 classes are a better choice than Calendar, but let's keep it simple for this answer.

Now you can write

private String timeDifferenceString(Calendar first, Calendar second, boolean isPast) {
    final String agoOrFromNow = isPast ? "ago" : "from now";
    final String lastOrNext = isPast ? "last " : "next ";

    final int years = second.get(Calendar.YEAR) - first.get(Calendar.YEAR);
    if (years > 1) {
        return years + " years " + agoOrFromNow;
    } else if (years > 0) {
        return lastOrNext + "year";
    }

    final int months = second.get(Calendar.MONTH) - first.get(Calendar.MONTH);
    if (months > 1) {
        return months + " months " + agoOrFromNow;
    } else if (years > 0) {
        return lastOrNext + "month";
    }

    ...

    return "now";
}

This leads to a rather lengthy but trivial method. Now you may want to split it as suggested by h.j.k. This is not easy, as the parts above only conditionally return something. Using a guard condition, this can be solved like this

    if (years > 0) {
        if (years > 1) {
            return years + " years " + agoOrFromNow;
        } else if (years > 0) {
            return lastOrNext + "year";
        } else {
            return "this year";
        }
    }

After extracting the method, you'll see that it's about the same for every time unit, so you may want to generalize it to

private String timeDifferenceString(int count, String unitName, boolean isPast) {
    if (count > 1) {
        return count + " " + unitName +  "s " + agoOrFromNow(isPast);
    } else if (count > 0) {
        return lastOrNext(isPast) + " " + unitName;
    } else {
        return "this " + unitName;
    }
}

Note that this won't work for other languages for many reasons including pluralization rules.

Now, you have a few trivial methods and one important one looking like

private String timeDifferenceString(Calendar first, Calendar second, boolean isPast) {
    final int years = second.get(Calendar.YEAR) - first.get(Calendar.YEAR);
    if (years > 0) {
        return timeDifferenceString(years, "year", isPast);
    }

    final int months = second.get(Calendar.MONTH) - first.get(Calendar.MONTH);
    if (months > 0) {
        return timeDifferenceString(months, "month", isPast);
    }

    ...

    return "now";
}

This is the right time for making changes, allowing you to output "48 minutes from now" instead of "next year" in my above example.


Code to initialize a .NET object from a CSV file I am initializing the following object from a CSV file:

private class MassUploadUser
    {
        public string email { get; set; }

        public string firstName { get; set; }

        public string lastName { get; set; }

        public string role { get; set; }

        public bool active { get { return true; } }
    }

I later serialize this to JSON using Newtonsoft, and then Post this to a RESTful API. TextFieldParser is from Microsoft.VisualBasic.FileIO by the way.

using (var parser = new TextFieldParser(Program.file))
{
    parser.SetDelimiters(",");

    string[] header = parser.ReadFields();

    while (!parser.EndOfData)
    {
            string[] fields = parser.ReadFields();

            var massUploadUser = new MassUploadUser();
            string password = null;

            for (int i = 0; i < fields.Length; i++)
            {
                switch (header[i])
                {
                    case "First Name":
                        massUploadUser.firstName = fields[i];
                        break;

                    case "Last Name":
                        massUploadUser.lastName = fields[i];
                        break;

                    case "Role":
                        massUploadUser.role = fields[i];
                        break;

                    case "email":
                        massUploadUser.email = fields[i];
                        break;

                    case "Password":
                        password = fields[i];
                        break;
                }
            }

           // After the for loop, I have some additional logic
           // to serialize the newly-created object to JSON
           // and then POST it to a RESTful API
           DoPostToWebService(massUploadUser, password);

          // At this point, we created the new user on the server, so no need
          // to keep the current instance of massUploadUser around
        }

Can this be improved? Is there a more "general" way to do this?

Here is another answer, using Reflection, Expression and Attribute to create a more automated solution.

First you create a couple of custom Attributes to mark fields or properties with the CSV Header Name when it doesn't match the class member name, or to indicate a class member isn't initialized from the CSV:

[AttributeUsage(AttributeTargets.Field | AttributeTargets.Property, AllowMultiple = false)]
public class CSVColumnName : Attribute {
    public string ColumnName { get; }
    public CSVColumnName(string name) => ColumnName = name;
}

[AttributeUsage(AttributeTargets.Field | AttributeTargets.Property, AllowMultiple = false)]
public class CSVNoColumnName : Attribute {
}

Then you can create a static method to build the PropertySetters Dictionary for you by Reflection. You need a couple of extension methods to make MemberInfo easier to work with:

public static Type GetMemberType(this MemberInfo member) {
    switch (member.MemberType) {
        case MemberTypes.Field:
            return ((FieldInfo)member).FieldType;
        case MemberTypes.Property:
            return ((PropertyInfo)member).PropertyType;
        case MemberTypes.Event:
            return ((EventInfo)member).EventHandlerType;
        default:
            throw new ArgumentException("MemberInfo must be if type FieldInfo, PropertyInfo or EventInfo", "member");
    }
}

public static bool GetCanWrite(this MemberInfo member) {
    switch (member.MemberType) {
        case MemberTypes.Field:
            return true;
        case MemberTypes.Property:
            return ((PropertyInfo)member).CanWrite;
        default:
            throw new ArgumentException("MemberInfo must be if type FieldInfo or PropertyInfo", "member");
    }
}

Then you can create the setter lambdas for each field and put them in a Dictionary:

public static class CSVMapping<T> {
    public static Dictionary<string, Action<T, object>> PropertySetters() {
        var t = typeof(T);
        var propsOrFields = t.GetMembers(BindingFlags.Instance | BindingFlags.Public).Where(m => m.MemberType == MemberTypes.Property || m.MemberType == MemberTypes.Field);

        var ans = new Dictionary<string, Action<T, object>>(StringComparer.OrginalIgnoreCase);
        foreach (var m in propsOrFields) {
            if (!Attribute.IsDefined(m, typeof(CSVNoColumnName)) && m.GetCanWrite()) {
                var ca = (CSVColumnName)Attribute.GetCustomAttribute(m, typeof(CSVColumnName));
                var csvname = (ca != null) ? ca.ColumnName : m.Name;
                // (T p1, object p2) => p1.{m.Name} = ({m.Type})p2;
                var paramobj = Expression.Parameter(t);
                var paramval = Expression.Parameter(typeof(object));
                var body = Expression.Assign(Expression.PropertyOrField(paramobj, m.Name), Expression.Convert(paramval, m.GetMemberType()));
                var setter = Expression.Lambda<Action<T, object>>(body, new[] { paramobj, paramval });
                ans.Add(csvname, setter.Compile());
            }
        }
        return ans;
    }
}

Note: You could use reflection to assign values as well, but I figure it is worth the effort to build the lambdas since you presumably will be assigning to the fields often (once per CSV file row).

Now you can annotate the class with CSV information:

private class MassUploadUser {
    public string email { get; set; }

    [CSVColumnName("First Name")]
    public string firstName { get; set; }

    [CSVColumnName("Last Name")]
    public string lastName { get; set; }

    public string role { get; set; }

    [CSVNoColumnName]
    public bool active { get { return true; } }
}

Note: Since active is a read-only property, the PropertySetters method would skip it even if it didn't have the attribute.

Finally, you can convert the CSV file to class members just like my previous answer, using the setters Dictionary to assign read-in values:

var fieldMap = CSVMapping<MassUploadUser>.PropertySetters();

using (var parser = new TextFieldParser(csvFilename)) {
    parser.SetDelimiters(",");

    string[] header = parser.ReadFields();
    var headerMap = header.Select((h, i) => new { h, i }).ToDictionary(hi => hi.h, hi => hi.i);

    while (!parser.EndOfData) {
        string[] fields = parser.ReadFields();

        var massUploadUser = new MassUploadUser();
        foreach (var field in fieldMap.Keys)
            fieldMap[field](massUploadUser, fields[headerMap[field]]);
    }
}


Fetching / formatting / storing tweets to my database I am periodically getting tweets, formatting them, and storing them into the database. Now the line of code I use for this is an awful lot. I was wondering if this could be improved.

// This is the wordpress way of getting a json object
// Wordpress will determine how the data is going to
// be fetched looking at what PHP functions are
// enabled for the user, wget, curl, etc.
$json_body = wp_remote_retrieve_body(
    wp_remote_get(
        'http://twitter.com/statuses/user_timeline/'.$options['username'].'.json?count='.$options['count'] ) );

$tweet = json_decode( $json_body, true );
for( $i = 0; $i < $options['count']; $i++ ){
  $latestTweet = htmlentities($tweet[$i]['text'], ENT_QUOTES);
  $latestTweet = preg_replace('/http:\/\/([a-z0-9_\.\-\+\&\!\#\~\/\,]+)/i', 'http://$1', $latestTweet);
  $latestTweet = preg_replace('/@([a-z0-9_]+)/i', '@$1', $latestTweet);

  // This array $data will be stored to the DB
  // I will use the *serialize()* function for this.
  $data[] =  array( 'text' => $latestTweet, 'time' => timespan( $tweet[$i]['created_at'] ) );
}

function timespan( $a )
{
  //get current timestampt
  $b = strtotime("now");
  //get timestamp when tweet created
  $c = strtotime($a);
  //get difference
  $d = $b - $c;
  //calculate different time values
  $minute = 60;
  $hour = $minute * 60;
  $day = $hour * 24;
  $week = $day * 7;

  if(is_numeric($d) && $d > 0) {
    //if less then 3 seconds
    if($d < 3) return "right now";
    //if less then minute
    if($d < $minute) return floor($d) . " seconds ago";
    //if less then 2 minutes
    if($d < $minute * 2) return "about 1 minute ago";
    //if less then hour
    if($d < $hour) return floor($d / $minute) . " minutes ago";
    //if less then 2 hours
    if($d < $hour * 2) return "about 1 hour ago";
    //if less then day
    if($d < $day) return floor($d / $hour) . " hours ago";
    //if more then day, but less then 2 days
    if($d > $day && $d < $day * 2) return "yesterday";
    //if less then year
    if($d < $day * 365) return floor($d / $day) . " days ago";
    //else return more than a year
    return "over a year ago";
  }
}

wp_remote_get(
        'http://twitter.com/statuses/user_timeline/'
        . $options['username'].'.json?count='.$options['count'] )

I'd use at least a local variable which stores the url:

$url = 'http://twitter.com/statuses/user_timeline/' .
    $options['username'] . '.json?count=' .$options['count'];
wp_remote_get($url);

A function would be more better. It makes the code easier to read.

Use longer variable names to avoid comments:

function timespan($create_time_input)
{
    $current_timestamp = strtotime("now");
    $create_time = strtotime($create_time_input);
    $diff_seconds = $current_timestamp - $create_time;

    $one_minute = 60;
    $one_hour = $one_minute * 60;
    $one_day = $one_hour * 24;
    $one_week = $day * 7;

    if(is_numeric($d) && $d > 0) {
        if ($diff_seconds < 3) {
            return "right now";
        }
        if ($diff_seconds < $one_minute) {
            return floor($diff_seconds) . " seconds ago";
        }
...

Now the code itself says what the comments said before.

Unnecessary floor:

if($d < $minute) return floor($d) . " seconds ago";

$d looks an integer, so floor is unnecessary.


Best way to toggle a property in an array I am runing this function that finds an object in an array and toggles the boolean lead property.
I am actually doing this in react so in there I do change the data via state and in the end if I run it again it would toggle Gary true/false, so it works fine.
My question is regarding the JavaScript code itself, it feels a bit convoluted, is there a better way?

const people = [
  {
    "name": "Olivia",
    "lead": true
  },
  {
    "name": "Gary",
    "lead": false
  },
  {
    "name": "Adam",
    "lead": false
  }
]

const toggleLead = (name) => {

  const values = [...people].map(person =>
    person.name === name
      ? {
          ...person,
          lead: !person.lead
        }
      : { ...person }
  )

  return values
}

console.log(toggleLead('Gary'))
.as-console-wrapper { max-height: 100% !important; top: 0; }

This could be accomplished via an XOR,  or boolean not equal in other words.

const people = [
  {
    "name": "Olivia",
    "lead": true
  },
  {
    "name": "Gary",
    "lead": false
  },
  {
    "name": "Adam",
    "lead": false
  }
]

const toggleLead = (name) =>
    [...people].map(person =>
        ({
            ...person,
            lead: person.lead !== (person.name===name)
        })
    )

console.log(toggleLead('Gary'))
.as-console-wrapper { max-height: 100% !important; top: 0; }


Mail utility class for use with Flask from application import app, mail
from threading import Thread
from flask.ext.mail import Message

__all__ = ['send_email', 'send_bulk_email', 'create_email']

def send_async_email(app, msg):
    with app.app_context():
        mail.send(msg)

def send_bulk_async_email(app, emails):
    with app.app_context():
        with mail.connect() as conn:
            for email in emails:
                conn.send(email)

def create_email(subject, recipients, text_body, html_body, sender=('My Company', 'info@mycompany.com.au')):
    email = Message(subject, sender=sender, recipients=recipients)
    email.body = text_body
    email.html = html_body

    return email

def send_email(email):
    thr = Thread(target=send_async_email, args=[app, email])
    thr.start()

def send_bulk_email(emails):
    thr = Thread(target=send_bulk_async_email, args=[app, emails])
    thr.start()

The first two functions are used internally as the threaded function calls. The create_email function is used to generate Message objects, which are then passed back to the send_email or send_bulk_email function dependent on whether there is a singular or multiple emails, respectively.

I currently think merging the singular and multiple send functions would be overkill, and remove some of the readability, however I am also open to feedback on that opinion.

I'd unify both functions and only have the bulk/multiple-emails version
because the single case is basically a specialisation of the general
case and the slight bit more work is unlikely to be noticeable (unless I
missed some other technical reason why this isn't feasible).

Btw. the with can be nested on a single line, i.e.:

def send_bulk_async_email(app, emails):
    with app.app_context(), mail.connect() as conn:
        for email in emails:
            conn.send(email)

The default value for the sender on create_email sounds like it
doesn't belong there and should rather be passed in like the rest,
perhaps store it in a configuration file anyway.

For spawning the threads also consider making it a bit more flexible by
allowing an optional parameter for the thread creation, e.g. to use a
thread pool instead of spawning an unlimited number of threads like done
here.


Updating a textarea label with the number of characters inside the field itself How would you improve and shorten this small piece of code? It basically update a textarea label with the number of characters inside the field itself. It saves the original label and restores it when characters count is 0.

jsFiddle here while sprintf is from here.

$('#send_sms_content').keyup(function() {

    // The label and characters count of textarea
    var label = $(sprintf('label[for="%s"]', $(this).attr('id'))),
        count = $(this).val().length;

    // Make backup of label original html into "data-original-value" attribute
    if($.type(label.attr('data-original-value')) === 'undefined') {
        label.attr('data-original-value', label.html());
    }

    // When empty textarea then restore the orignal label
    if(count == 0) {
        label.html(label.attr('data-original-value'));
        return;
    }

    // Regular expression to match any digit in the label
    var digit = new RegExp("[0-9]+")

    // If there are no digits in the label append the count
    if(!digit.test(label.html())) {
        label.html(label.html() + ' ' + sprintf('(%s)', count));
    }
    else { // There are digits already, replace with the new count
        label.html(label.html().replace(digit, count));
    }

});

Just a few things I notice, prolly not warranting a "mark as answer" but may help:

Cache $(this): var $this = $(this); So that you don't keep inefficiently re-wrapping this
$(this).attr('id') - really? You can just use this.id, you know.
hasDigit === digit, so why do you have both? Scrap one of those regexes, the other one is redundant.
Cache label.html() so that you don't keep grabbing it (which is quite inefficient). Instead: var labelHtml = label.html() and then reuse labelHtml


Is this a good way to track current site being reported on? I'm trying to find an elegant and secure way to track the currently reported site. I originally just used a cookie but realized this was not very secure. I have since switched to combine the session and the cookie. It still feels a bit clumsy. Am I missing something here as far as best practices? Can this be improved on?

public static int SiteID
        {
            get
            {
                if (HttpContext.Current.Session[SITE_ID_KEY] == null)
                {
                    if (Web.CookieUtil.CookieExists(SITE_ID_KEY))
                    {
                        Guid userId = (Guid)Membership.GetUser().ProviderUserKey;
                        int siteId = Convert.ToInt32(Web.CookieUtil.GetCookieValue(SITE_ID_KEY));
                        if (Sites.ValidateSite(StateManager.AccountID.Value, siteId, userId))
                        {
                            HttpContext.Current.Session[SITE_ID_KEY] = siteId;
                        }
                        else
                        {
                            Web.CookieUtil.DeleteCookie(SITE_ID_KEY);
                            return -1;
                        }
                    }
                    else
                    {
                        return -1;
                    }
                }

                return (int)HttpContext.Current.Session[SITE_ID_KEY];
            }
            set
            {
                HttpContext.Current.Session[SITE_ID_KEY] = value;
                Web.CookieUtil.CreateCookie(SITE_ID_KEY, value.ToString(), null);
            }
        }

As you pointed out, relying on cookies means that the user can choose someone else's site. That means you should not use cookies at all, not even in addition to the session. If you choose to use cookies anyway, you have to validate that the current user actually has access to the selected site before returning any data (or even worse, before performing some action), which your code already does. You also have to be very careful that there isn't any code path that doesn't perform the validation.

Because of this, I think you should store the data in session only. If you need more persistent storage, use your database, not cookies.

Some points about your code:

return -1;

You shouldn't use “magic” values like -1 to indicate “no value”. Instead, change the return type to int? and return null of no site is selected.

Convert.ToInt32(…)
value.ToString()

This may not work well unless you're 100 % sure that all servers running this code will be on the same culture. You should probably use InvariantCulture here.

Also, if the cookie contains garbage, I think your code should return “no value” not throw an exception.


Filter then map instances from a stream and other Java 8 hiccups I'm having a hard time rewriting this method to something more legible and understandable for the reader.

static Stream<Class<? extends MyType>> collectMyTypes(Object o) {
  return Reflection.lineage(o.getClass())
      .flatMap(c -> Arrays.stream(c.getDeclaredFields()))
      .map(Field::getGenericType)
      .filter(t -> t instanceof ParameterizedType)
      .map(t -> (ParameterizedType) t)
      .filter(t -> t.getRawType() == MyTypeWrapper.class)
      .map(t -> t.getActualTypeArguments()[0]) //MyTypeWrapper is final and has only 1 type argument.
      .filter(t -> t instanceof Class)
      .map(t -> (Class<? extends MyType>) t);
}

Here are the tools/classes I use:

Reflection.lineage(Class<T>) iterates over all parents classes of T;
MyTypeWrapper is defined as final class MyTypeWrapper<T extends MyType>;
MyType is defined as abstract class MyType.

What annoys me with this code is that it's barely legible, mostly because of the succession of filter on instance then map. I tried to find in Java 8  something similar to Guava's Iterables.filter(Iterable<?>,Class<T>) but haven't found any (yet, I implemented a similar method, but the code then looks like Lisp). Also, the call to Arrays.stream disturbs me. Yeah, I could split it into .map(Class::getDeclaredFields).flatten(Arrays::stream) but this would add some unnecessary complexity, wouldn't it?

Anyways, any input is welcome.

Given the example below, this method will keep/skip various fields.

class GoodExample extends MyType {}
class BadExample extends MyType {}
class Example {

  // GoodExample will be returned.
  MyTypeWrapper<GoodExample> object1;

  // BadExample will be skipped because the type is a wildcard
  MyTypeWrapper<? extends BadExample> object2;

  // There is no type to return so this field is skipped.
  MyTypeWrapper object3;

  // This is not an instance of MyTypeWrapper so it's skipped.
  Object object4;

  // This is generic, but still not a MyTypeWrapper, so it's skipped.
  List<BadExample> object5;
}

Edit:

Originally the method was this:

  List<Class<? extends MyType>> types = new ArrayList<>();
  Reflection.lineage(o.getClass())
      .forEach((c) -> {
        for (Field f: c.getDeclaredFields()) {
          Type type = f.getGenericType();
          if (!(type instanceof ParameterizedType)) {
            continue;
          }
          ParameterizedType pType = (ParameterizedType)type;
          if (pType.getRawType() != MyTypeWrapper.class) {
            continue;
          }
          // length is 1.
          Type argType = pType.getActualTypeArguments()[0];
          if (!(argType instanceof Class)) {
            continue;
          }
          Class<? extends MyType> argClass = (Class<? extends MyType>)argType;
          types.add(argClass);
        }
      });
return types;

What I can suggest is putting the subsequent filter()/map() steps into an extraction method of sorts:

@SuppressWarnings("rawtypes")
private static Optional<Type> extractMyType(Field field) {
    return Optional.of(field.getGenericType())...;
}

You can also put the filter()/map() steps as a single method:

private static <T, R> Optional<R> filterAndMap(T obj, Predicate<? super T> filter,
        Function<? super T, ? extends R> mapper) {
    return Optional.of(obj).filter(filter).map(mapper);
}

For convenience, you can create a cast() method that calls filterAndMap() to handle safe-casting:

private static <T, R> Optional<R> cast(T obj, Class<R> clazz) {
    return filterAndMap(obj, clazz::isInstance, v -> (R) obj);
}

Then, the meat of extractMyType() effectively becomes three flatMap() operations:

@SuppressWarnings("rawtypes")
private static Optional<Class> extractMyType(Field field) {
    return Optional.of(field.getGenericType())
            .flatMap(v -> cast(v, ParameterizedType.class))
            .flatMap(v -> filterAndMap(v,
                            v1 -> v1.getRawType() == MyTypeWrapper.class,
                            v1 -> v1.getActualTypeArguments()[0]))
            .flatMap(v -> cast(v, Class.class));
}

Finally, your collectMyTypes() method will just do a simple filter on the resulting Optional wrapper instances:

@SuppressWarnings("unchecked")
static Stream<Class<? extends MyType>> collectMyTypes(Object o) {
    return Reflection.lineage(o.getClass())
            .flatMap(c -> Arrays.stream(c.getDeclaredFields()))
            .map(YourClass::extractMyType).filter(Optional::isPresent)
            .map(Optional::get);
}


Finding the longest common substring of multiple strings in Haskell The task is to find the longest common substring of a multitude of long strings. I'm trying to find the best algorithm that wouldn't use syntax trees/arrays (as I don't know anything about them yet).

The idea was to use binary search to find the length of the desired substring, instead of simply trying n, then (n - 1) etc.

My code looks like this:

-- find all the substrings of length n
ngrams :: Int -> String -> [String]
ngrams n s | n > length s = []
           | otherwise    = take n s : ngrams n (drop 1 s)

-- find the longest common substring of multiple strings
longestCommonSubstring :: [String] -> String
longestCommonSubstring xs = go 0 $ length (head xs) + 1
 where
  -- find a substring of a given length n that is common to all strings
  commonSubstrings n = foldr1 intersect (map (ngrams n) xs)
  go l r
    -- if the binary search ended, pick one common substring
    | r - l == 1 = head $ commonSubstrings l
    | otherwise
    = case commonSubstrings m of
      [] -> go l m    -- the length is too big, try a smaller one
      _  -> go m r    -- try longer length to find longer substring
   where
    m = (l + r) `div` 2    -- the middle point

It runs at around 3s for my dataset (~100 strings of length ~1000), which seems slow to me. Is there any way to clean up and quicken the code? And is there a better way to approach this problem (apart from the syntax trees) in general?

intersect runs in quadratic time. Sets can use Ord information to speed that up.

-- find the longest common substring of multiple strings
longestCommonSubstring :: [String] -> String
longestCommonSubstring xs = go 0 $ length (head xs) + 1
 where
  -- find a substring of a given length n that is common to all strings
  commonSubstrings n = foldr1 S.intersection (map (S.fromList . ngrams n) xs)
  go l r
    -- if the binary search ended, pick one common substring
    | r - l == 1 = S.findMin $ commonSubstrings l
    | otherwise
    = if S.null $ commonSubstrings m
      then go l m    -- the length is too big, try a smaller one
      else go m r    -- try longer length to find longer substring
   where
    m = (l + r) `div` 2    -- the middle point

For comparison, here's an implementation that skips the binary search part:

-- find the longest common substring of multiple strings
longestCommonSubstring :: [String] -> String
longestCommonSubstring = maximumBy (comparing length)
  . foldr1 S.intersection . map (S.fromList . map inits . tails)


Avoid explicitly writing base class state in a derived class. Suggestions for my method I have two classes one base Foo class and one derived Bar class.

Each class has its own associated settings class that can create a new instance from the Load method.  This way all derived class settings can be stored in one list of FooSettings.

public abstract class FooSettings
{
    public int a;
    public int b;
    public float c;

    public abstract Foo Load();
}

public class BarSettings : FooSettings
{
    public float d;

    public override Foo Load()
    {
        return new Bar(this);
    }
}

The classes themselves have their settings as an argument for the constructor and are created from their associated settings class (null = default settings class state)

public abstract class Foo
{
    private int a = 0;
    private int b = 1;
    private float c = 1.2f;

    public Foo() { }

    public abstract FooSettings WriteSettings();

    protected void WriteBaseFooSettings(ref FooSettings settings)
    {
        settings.a = a;
        settings.b = b;
        settings.c = c;
    }

    public int A
    {
        get { return a; }
        set { a = value; }
    }

    public int B
    {
        get { return b; }
        set { c = value; }
    }

    public float C
    {
        get { return c; }
        set { c = value; }
    }
}

public class Bar : Foo
{
    float d = 0.9f;

    public Bar()
        : base() { }

    // METHOD A
    public override BaseFooSettings WriteSettings()
    {
        DerivedBarSettings settings = new DerivedBarSettings();

        settings.a = A;
        settings.b = B;
        settings.c = C;
        settings.d = d;

        return settings;
    }

    // METHOD B
    public override FooSettings WriteSettings()
    {
        var settings = new BarSettings();

        // Write base settings
        var baseSettings = settings as FooSettings;
        WriteBaseFooSettings(ref baseSettings);

        // Write derived/local settings
        settings.d = d;

        return settings;
    }
}

The problem with this setup is that I found it somewhat wasteful to have to rewrite the base class settings in the derived class method each time I wrote the derived class settings (METHOD A).

In order to get around this problem I went with METHOD B but I am not certain as to whether this is a good solution.  Some advice on improving METHOD B or a better alternative would be welcome.

I think this is the right way to do it. If you have some code that should be used from the base class and all its derived classes, it should be in the base class. Which is exactly what you're doing.

But I would make a modification in your code. You don't need to use ref. In C#, classes are reference types, which means that the values in the object will change, even if you don't use ref. And if you remove the ref, you won't need the baseSettings variable anymore, making the code shorter:

protected void WriteBaseFooSettings(FooSettings settings)
{
    // set the properties, same as before
}

…

public override FooSettings WriteSettings()
{
    var settings = new BarSettings();

    WriteBaseFooSettings(settings);

    // Write derived/local settings
    settings.d = d;

    return settings;
}


Counting last reported version in shell Struggling to write a simple tool to quickly sum up the last reported versions in a bunch of files. I'm using a named pipe to get the count out the loop for the final value, but the $version isn't populated which I don't understand.

mkfifo mypipe
for i in stats/*
do
    tail -1 $i
done | sort -n > mypipe &

while read version
do

    if test "$version" = "$last"
    then
        count=$(($count + 1))
    else
        echo $last $count
        count=1
        last=$version
    fi

done < mypipe

# $version is not printed, what's the workaroud?
echo version $version count $count

Perhaps people can suggest a better cleaner more suck less solution?

Your immediate problem is that the exit condition of the loop is when read version returns an error status. This happens when the loop reaches the end of its input file. This call to read also sets version to the empty string.

You can salvage this code by keeping the last known-good value of version in another variable — which you already do here, in last. So change the last line to echo version $last count $count. But there are simpler ways of doing this; uniq -c, which has already been suggested being the simplest, and awk as a general purpose text processing tool being something to keep in mind as well.

While I'm at it, some general notes on shell programming.

Always put double quotes around variable substitutions and command substitutions, unless you can explain why it is safe to leave them off in a particular instance. For example, tail -1 $i should be tail -1 "$i": if you leave off the quotes, this command will fail if the file name contains whitespace or globbing characters.
In fact, this line should be

tail -n 1 -- "$i"

The -- is there in case $i begins with a -; without the -- it would be treated as an option to tail. In this specific case $i always begins with s so the -- is unnecessary, but it's a good habit to get into.
The form -1 is obsolete, and some modern implementations of tail don't support it. -n 1 is the standard form.
To read a line from a file, don't use plain read, but IFS= read -r. See How can I read line by line from a variable in bash? Plain read expands backslashes (expecting a continuation line if a line ends with a \), and strips off leading and trailing whitespace.
Using a named pipe requires a lot of management: you need to create it with a unique name (use mktemp) and make sure the permissions are correct. You may have tried and failed with the following structure:

for i in stats/*; do … done | while read version; do … count=…; done
echo $count

In most shells (Bourne, ash, bash, pdksh), each component of a pipe is executed in its own subshell, so any assignment to count in the while loop doesn't affect the parent shell. Only in a few shells (ATT ksh, zsh) does the rightmost command in a pipeline run in the parent shell. There's a simple workaround for that: run all the code that depends on the assignments performed by a pipeline component inside that pipeline component.

for i in stats/*; do … done | sort -n | {
  while read version; do … count=…; done;
  echo $count
}


Powershell script to verify Linux-generated md5sum file I'm moving some files from a Linux machine to a Windows machine over a low-speed, possibly buggy experimental communications channel that I want to test.  One of the tests is to transfer large numbers of large and small files and verify their cryptographic hashes at the receiving end.  On the Linux side, we're using md5sum to generate file hashes like so:
md5sum * > files.md5

Then the files are transmitted from the Linux machine to the Windows 10 machine.  What I'd like to do next is to verify the hashes on the plain-vanilla Windows machine (no Cygwin installed). So to mimic the default operation of md5sum -c files.md5 which would go through, line by line and verify each md5 checksum, I've written this Powershell script.  I'm a lot more at home in bash than in Powershell, so thought I might benefit from a review.
param (
    [Parameter(Mandatory=$true)][string]$infile
)
$basedir = Split-Path -Parent $infile
$badcount = 0
foreach ($line in [System.IO.File]::ReadLines("$infile")) {
    $sum, $file = $line.split(' ')
    $fullfile = "$basedir\$file"
    $filehash = Get-FileHash -Algorithm MD5 $fullfile
    if ($sum -eq $filehash.Hash) {
        Write-Host $file ": OK"
    } else {
        Write-Host $file ": FAILED"
        $badcount++
    }
}
if ($badcount -gt "0") {
    Write-Host "WARNING:" $badcount "computed checksums did NOT match"
}

here are a few changes i would make. [grin] the ideas ...

use Get-Content instead of ReadLines()
the speed difference is not large unless you are dealing with a very large number of files. go with the standard cmdlets unless there is a meaningful benefit from doing otherwise.
test to see if the file exists
build a [PSCustomObject] to hold the resulting items that you want
keep those PSCOs in a collection
view your hash failure items after the full test ends

what it does ...

sets the constants
builds a test file to work with
remove the entire #region/#endregion block when you are ready to use your own data.
reads in the hash list file
iterates thru the resulting array
splits out the file name and hash value
builds the full file name to check
tests to see if that file exists
if YES, gets the file hash and saves it
if NO, sets the file hash $Var to '__N/A__'
builds a PSCO with the properties that seem useful
sends that to the $Result collection
gets the hash failures from the collection and displays them
if all you want it the count, wrap that all in @() and add .Count to the end.

the code ...
$SourceDir = $env:TEMP
$HashFileName = 'FileHashList.txt'
$FullHashFileName = Join-Path -Path $SourceDir -ChildPath $HashFileName

#region >>> make a hash list to compare with
#    remove this entire "#region/#endregion" block when ready to work with your real data
$HashList = Get-ChildItem -LiteralPath $SourceDir -Filter '*.log' -File |
    ForEach-Object {
        '{0} {1}' -f $_.Name, (Get-FileHash -LiteralPath $_.FullName-Algorithm 'MD5').Hash
        }
# munge the 1st two hash values
$HashList[0] = $HashList[0] -replace '.{5}$', '--BAD'
$HashList[1] = $HashList[1] -replace '.{5}$', '--BAD'

$HashList |
    Set-Content -LiteralPath $FullHashFileName
#endregion >>> make a hash list to compare with

$Result = foreach ($Line in (Get-Content -LiteralPath $FullHashFileName))
    {
    $TestFileName, $Hash = $Line.Split(' ')
    $FullTestFileName = Join-Path -Path $SourceDir -ChildPath $TestFileName
    if (Test-Path -LiteralPath $FullTestFileName)
        {
        $THash = (Get-FileHash -LiteralPath $FullTestFileName -Algorithm 'MD5').Hash
        }
        else
        {
        $THash = '__N/A__'
        }
    [PSCustomObject]@{
        FileName = $TestFileName
        CopyOK = $THash -eq $Hash
        OriHash = $Hash
        CopyHash = $THash
        }
    }

$Result.Where({$_.CopyOK -eq $False})

output [with the 1st two hash values deliberately munged] ...
FileName                  CopyOK OriHash                          CopyHash
--------                  ------ -------                          --------
Genre-List_2020-07-07.log  False 7C0C605EA7561B7020CBDAE24D1--BAD 7C0C605EA7561B7020CBDAE24D140E40
Genre-List_2020-07-14.log  False 20F234ACE66B860821CF8F8BD5E--BAD 20F234ACE66B860821CF8F8BD5EC144D


Allow UITextView to resize as the user is typing I want to be sure that in terms of auto-layout or just following the proper methods of doing the following is acceptable. I am not sure if there will be an constraint issues or efficiency problems that could slow down my app. It is working so far, I am in the process of trying to animate it as it resizes.

import UIKit

class PickUpViewController: UIViewController, UITextViewDelegate {

    @IBOutlet weak var addressTextVIew: TextViewAutoHeight!
    override func viewDidLoad() {
        super.viewDidLoad()
        addressTextVIew.delegate = self
    }

    func textViewDidChange(_ textView: UITextView) {
        if textView.contentSize.height > textView.frame.size.height {

            let fixedWidth = textView.frame.size.width
            textView.sizeThatFits(CGSize(width: fixedWidth, height: CGFloat.greatestFiniteMagnitude))

            var newFrame = textView.frame
            let newSize = textView.sizeThatFits(CGSize(width: fixedWidth, height: CGFloat.greatestFiniteMagnitude))

            newFrame.size = CGSize(width: max(newSize.width, fixedWidth), height: newSize.height)

            textView.frame = newFrame;
        }
    }
}

It can actually be a lot simpler!

UITextView is a UIScrollView where by default the contentSize updates to fit whatever text is in the current view.

Assuming that you have constraints set up (e.g. pinning left, top, and right anchors so that the height is free to grow) then it's as simple as overriding the default value of isScrollEnabled and setting it to false.

You could do that either in your storyboard, or in your text view subclass.

I would suggest moving any logic you have around animating size of the text view out of the view controller and bundle it into your text view subclass.

Assuming that your constraints are set up in the storyboard, your view controller (at this point) doesn't need to have anything:

import UIKit

class ViewController: UIViewController {

}

Your text view subclass should have everything it needs to do to animate it's height change, and keeping the code there will make your view controllers more lightweight, and your TextViewAutoHeight more reusable in different scenarios.

Your current code doesn't actually animate the height so it's off-topic for /CodeReview, so I suggest searching stack overflow for questions around "UIView animate with duration", and perhaps reposting here when you have working code ready for review.

import UIKit

class TextViewAutoHeight: UITextView {

    required init?(coder aDecoder: NSCoder) {
        super.init(coder: aDecoder)
        isScrollEnabled = false
        NotificationCenter.default.addObserver(self, selector: #selector(updateHeight), name: NSNotification.Name.UITextViewTextDidChange, object: nil)
    }

    func updateHeight() {
        // trigger your animation here

        /*
        var newFrame = frame

        let fixedWidth = frame.size.width
        let newSize = sizeThatFits(CGSize(width: fixedWidth, height: CGFloat.greatestFiniteMagnitude))

        newFrame.size = CGSize(width: max(newSize.width, fixedWidth), height: newSize.height)
        self.frame = newFrame
        */

        // suggest searching stackoverflow for "uiview animatewithduration" for frame-based animation
        // or "animate change in intrinisic size" to learn about a more elgant solution :)
    }
}


Showing a menu with buttons with dropdown submenus I have to display a menu where some buttons have dropdown submenus and show if the current button is active or not and put a CSS class to it for each of the two situations (or in both).

<?php

$menus_array = array(
    array('item1', false),
    array('item2', true),
    array('item3', false)
);

$active_item = 'item2';

foreach ($menus_array as $menu_item) {
    if ($menu_item == $active_item && $menu_item[1] != true) {
        $css_class = ' class="active"';
    } else if ($menu_item != $active_item && $menu_item[1] == true) {
        $css_class = ' class="dropdown"';
    } else if ($menu_item == $active_item && $menu_item[1] == true) {
        $css_class = ' class="active dropdown"';
    } else {
        $css_class = '';
    }
}

?>

I've written the code as above, but I don't really like it. Is there a way to write it better? I'm talking about the foreach loop.

A few things first:

$menu_item == $active_item should probably be $menu_item[0] == $active_item
Depending on where that array is coming from, it should probably be associative.  Imagine if you came across the code out of context.  You'd quickly find yourself wondering what in the world menu_item[0] and menu_item[1] are.  menu_item['name'] and menu_item['down'] would be much more descriptive (or something like that).
if (... && $x == true) {} is identical to if (... && $x) { }.  Don't include the implicit truth comparison.  The only time you should compare against booleans is if you're wanting to compare type too.  if ($result === false) {}

Anyway, I would probably do something like this:

foreach ($menus_array as $menu_item) {

    $classes = array();

    if ($menu_item[0] == $active_item) {
        $classes[] = 'active';
    }

    if ($menu_item[1]) {
        $classes[] = 'dropdown';
    }

    $css_class = ' class="' . implode(' ', $classes) . '"';

}

Or, if you are into micro-optimizing (and you shouldn't be):

foreach ($menus_array as $menu_item) {

    $class = '';

    if ($menu_item[0] == $active_item) {
        //Could be = instead of .= but putting .= will mean that you
        //can add another assignment above this and not have to change
        //= to .= here.
        $class .= 'active ';
    }

    if ($menu_item[1]) {
        $class .= 'dropdown ';
    }

    $css_class = ' class="' . rtrim($class) . '"';

}

(The rtrim really isn't necessary, but I'm picky. If I were super worried about performance, I would omit the rtrim.)

I would go with the array based one.  Rendering HTML is never going to be your bottleneck, and it's the easier one for me to read.


Object-oriented URL loading and HTTP client with requests in Python I have an API which I want to consume in Python 3.x using the requests module.
Right now, my solution is to create Requests objects for each URL like the LoginRequest, etc. (see below).

I have one request per API URL and need to write the __init__ method for each class depending on the parameters that are requested for the API. I find it tedious and I am not sure that's a good design.

Could you suggest a better design? (e.g. not using OOP, with metaclass, ... ?)

class BaseRequest:
    USER_AGENT = 'My App 1.0'
    headers = {'User-Agent': USER_AGENT}
    url = None
    data = None
    params = None

    def __init__(self, url, data=None, headers=None, params=None):
        self.url = url
        if data:
            data.update(self.data or {})
            self.data = data
        if headers:
            headers.update(self.headers or {})
            self.headers = headers
        if params:
            params.update(self.params or {})
            self.params = params

    def process(self):
        r = requests.post(self.url, data=self.data, headers=self.headers, params=self.params)
        try:
            j = r.json()
        except:
            print(self.url, self.params, self.data)
            print(r.text)
            exit()
        return j

class AuthenticatedRequest(BaseRequest):
    API_KEY = 'blah'
    APP_ID = 'OfficialClient'
    data = {'appkey': API_KEY, 'appid': APP_ID}

class LoginRequest(AuthenticatedRequest):
    url = 'https://my.api.com/login'

    def __init__(self, email, password):
        super().__init__(self.url, data=dict(email=email, password=password, action='login'))

# now let's play with the API !
user = LoginRequest(email, password).process()
GetUserInfoDetailsRequest(...).process()

It makes sense to model the API like this if things are more
complicated, e.g. ability to pass around requests and responses as
objects might come in handy.

The last example shows why this alone isn't that user-friendly though:
In addition to mentioning the specific request (with the Request
suffix always around) you also have to call the process method.

I'd suggest an additional wrapper to make things easier in the normal
case, i.e. have login function or method that does this for you.

The code as-is looks good to me with the exception of the process
method - firstly you can just write return r.json(), secondly the
exit call in there is ... questionable.  I most certainly wouldn't
want to have a library call that on my program ever.

A minor thing I've also noticed is that the subclasses of BaseRequest
can't override any values via the constructor.  I'm guessing that's
deliberate, I just usually expect to be able to do that (so in the base
class call self.headers.update(headers) instead of the other way
round).


Am I reconstructing the buffers correctly I am writing a networked audio application that sends the audio in 320 byte chunks, later I need to provide the player a bigger amount of bytes to play so I try to merge 2 or more chunks into one buffer and return it, well the code works but I hear some small audio artifacts, any ideas if the code is correct?

-(NSData*) getRawBufferWithSize:(int) requestedBufferSize
{
    if(audio.isBuffering && audio.bufferTimer>[[NSDate date] timeIntervalSince1970] && !AUDIO_BYPASS_NET)
    {
        return nil;
    }
    audio.isBuffering = NO;
    int readyBufferSize = 0;
    int bytesPending = requestedBufferSize;

    if(returnBufferSize<requestedBufferSize)
    {
        returnBuffer = (char*)realloc(returnBuffer, requestedBufferSize);
        returnBufferSize = requestedBufferSize;
    }

    while (readyBufferSize<requestedBufferSize)
    {
        NSData* newBuffer;
        @synchronized(playbackRawBuffer)
        {
            if([playbackRawBuffer count] != 0)
            {
                newBuffer = [[playbackRawBuffer objectAtIndex:0] retain];
                [playbackRawBuffer removeObjectAtIndex:0];
            }
            else
            {
                break;
            }
        }
        char* bytes = (char*)[newBuffer bytes];

        int readableBytes = newBuffer.length;
        int amountOfBytesToRead = bytesPending;
        int amountOfBytesToSaveForLater = 0;

        if(readableBytes<amountOfBytesToRead)//if we can read less than we want
        {
            amountOfBytesToRead = readableBytes;
        }
        else if(readableBytes>amountOfBytesToRead)//if we can read more bytes than we want (we save the remaining bytes)
        {
            amountOfBytesToSaveForLater = readableBytes - amountOfBytesToRead;
        }

        bytesPending -= amountOfBytesToRead;
        memcpy((char*)returnBuffer+readyBufferSize, bytes, amountOfBytesToRead);
        [newBuffer release];

        readyBufferSize+=amountOfBytesToRead;

        if(amountOfBytesToSaveForLater>0)
        {//save these bytes back in the buffers array
            if(tBufSize<amountOfBytesToSaveForLater)
            {
                tBuf = (char*)realloc(tBuf, amountOfBytesToSaveForLater);
                tBufSize = amountOfBytesToSaveForLater;
            }
            memcpy(tBuf, &(bytes[amountOfBytesToRead]), amountOfBytesToSaveForLater);
            @synchronized(playbackRawBuffer)
            {
                [playbackRawBuffer insertObject:[NSData dataWithBytes:tBuf length:amountOfBytesToSaveForLater] atIndex:0];
            }
            break;
        }
    }

    if(readyBufferSize>0)
    {
        return [NSData dataWithBytes:returnBuffer length:readyBufferSize];
    }
    else
    {
        audio.isBuffering = YES;
        audio.bufferTimer = [[NSDate date] timeIntervalSince1970]+0.1;
        return nil;
    }
}

This is the wrong way to sue realloc:

returnBuffer = (char*)realloc(returnBuffer, requestedBufferSize);

If it fails you have just leaked the old returnBuffer.
The way realloc should be used is:

char* tmp = (char*)realloc(returnBuffer, requestedBufferSize);
if (tmp != NULL)
{
    returnBuffer = tmp;
}
else { /* Deal with realloc() failure */ }

But why are you dealing with this. I would use a std::vector<char> it deals with all the memory management issues for you (and your post id tagged C++).

TO be honest I have trouble following the rest of your code.


Design a procedure to reverse a list SICP exercise 2.18 asks the following:

  Exercise 2.18.  Define a procedure
  reverse that takes a list as argument
  and returns a list of the same
  elements in reverse order:

(reverse (list 1 4 9 16 25))
(25 16 9 4 1)

I wrote this iterative function:

(define (reverse n)
  (define (iter a b)
    (if (null? a) b
        (iter (cdr a) (cons (car a) b))))
  (iter n null))

I could not think of a way to do this recursively.  Is there a better way to write this function?  Is it possible to do it recursively?

Here's a recursive definition of reverse (named rev) that uses primitives (car/cdr/cons) and an inner definition of snoc (reverse cons):

(define (rev lis)
  (define (snoc elem lis)
    (if (null? lis)
        (cons elem lis)
        (cons (car lis) (snoc elem (cdr lis)))))
  (if (null? lis)
      lis
      (snoc (car lis) (rev (cdr lis)))))

Obviously, this definition is not meant to be efficient. =)


find substring with count and return even frequency substring list I have one following question and write a function for it. can anyone give me some suggestion how my code can be better and better way to handle to build up return list with even frequency?

  One message system contains two device type message that each message is formatted with device type identifier with
  device id and message count. Write a function to parse the message based on device type and message count and return
  a list with each device id in even message frequency
  rule: input string: message
  device identifier
  1:iOS device ID identifier: start with 'I' following with 3 character, total length is 4 character
  2:Android device ID identifier: start with 'A' following with 2 character, total length is 3 character
  3:message count is following by device id until next device ID
  ex:
  input: Asq2: {'Asq': 2} Asq with 2 message count
  output: ['Asq', 'Asq']

  input: Akb2IAld3: ID: {'Akb': 2, 'IAld2': 3}  Akb with 2 message count,  IAld with 3 message count
  output: ['Akb', 'IAld', 'Akb', 'IAld', 'IAld']

  input: Aqp1Iasd2Aqp4IAbd1: {'Aqp': 5, 'Iasd': 2, 'IAbd': 1}
  output: ['Aqp', 'Iasd', 'IAbd', 'Aqp', 'Iasd', 'Aqp', 'Aqp', 'Aqp']

from typing import List

def parse_message(string) -> List:
    i, j, ids_map, n, ids = 0, 0, dict(), len(string), ''

    while i < n:
        if string[i] in ('I', 'A') or i == n - 1:
            if ids:
                if i == n - 1:
                    ids_map[ids] = ids_map.get(ids, 0) + int(string[j:])
                else:
                    ids_map[ids] = ids_map.get(ids, 0) + int(string[j:i])
            j = i + 4 if string[i] == 'I' else i + 3
            ids = string[i:j]
            i = j - 1
        i += 1
    res = []
    while any(i > 0 for i in ids_map.values()):
        for k, v in ids_map.items():
            if v > 0:
                res.append(k)
                ids_map[k] -= 1
    return res

Your code is really hard to understand. When I read the description I noticed three steps:

Extract ID and message count.
Total duplicate message counts.
Return a round robin of these IDs.

You've got 1&2 mangled together, and 3 nicely by itself.

Since you're using Python, it's easier to read iterator based approaches to these things. Firstly you can replace your step 3 with a recipe from the itertools standard library. All you need to do is call itertools.repeat beforehand.

It's not immediately clear what your code is doing when you've mangled indexed iteration with business logic, with step 2 as well. And so I suggest re-writing it. You know the data will come in the form {type}{message}{amount}, where the type and amount has a length of 1, and message is either 2 or 3. And so to extract the messages and amounts you can just use next/islice on an iterator. Using a try while loop. After this totaling the message counts is simple.

Also if you're going to use typing you should use mypy too, and put type information on both the arguments and return types.

from typing import List, Iterator, Tuple, Sequence, TypeVar
    import itertools

MESSAGE_LENGTH = {
    'I': 3,
    'A': 2
}

TValue = TypeVar('TValue')

def roundrobin(*iterables: Tuple[Sequence[TValue], ...]) -> Iterator[TValue]:
    "roundrobin('ABC', 'D', 'EF') --> A D E B F C"
    # Recipe credited to George Sakkis
    num_active = len(iterables)
    nexts = itertools.cycle(iter(it).__next__ for it in iterables)
    while num_active:
        try:
            for next in nexts:
                yield next()
        except StopIteration:
            # Remove the iterator we just exhausted from the cycle.
            num_active -= 1
            nexts = itertools.cycle(itertools.islice(nexts, num_active))

def extract_messages(input: Iterator[str]) -> Iterator[Tuple[str, int]]:
    input = iter(input)
    message_type = next(input)
    while True:
        length = MESSAGE_LENGTH[message_type]
        message = ''.join([message_type] + list(itertools.islice(input, length)))
        message_type = next(input)
        number = []
        try:
            while message_type not in MESSAGE_LENGTH:
                number += [message_type]
                message_type = next(input)
        except StopIteration:
            break
        finally:
            yield message, int(''.join(number))

def parse_message(string: str) -> List[str]:
    message_counts = {}
    for message, amount in extract_messages(string):
        message_counts.setdefault(message, 0)
        message_counts[message] += amount

    return list(roundrobin(*(
        itertools.repeat(key, amount)
        for key, amount in message_counts.items()
    )))

if __name__ == '__main__':
    print(parse_message('Akb2IAld3'))
    print(parse_message('Aqp1Iasd2Aqp4IAbd1'))
    print(parse_message('Aqp1Iasd2Aqp4IAbd10'))


Loop through a worksheet and insert a row with layout when value is true I have a code that does a loop through a worksheets, if there is a value 2 inside a cell in column S, then I want to insert a row with a specific layout. I have the code, but it takes ages to complete. I've tried replacing .select function, but because I need a specific layout, I don't know how to avoid this.

LastRowMatchC = Worksheets("Compliance").Cells(Rows.Count, 1).End(xlUp).Row

Dim rngc As Range, rc As Long

Set rngc = Range("S8:S" & LastRowMatchC)

For rc = rngc.Count To 1 Step -1
    If rngc(rc).Value = 2 Then
        rngc(rc + 1).EntireRow.Insert
        rngc(rc + 1).EntireRow.Select

    With Selection.Interior
        .Pattern = xlSolid
        .PatternColorIndex = xlAutomatic
        .ThemeColor = xlThemeColorAccent1
        .TintAndShade = 0.599993896298105
        .PatternTintAndShade = 0
    End With
    Selection.Borders(xlDiagonalDown).LineStyle = xlNone
    Selection.Borders(xlDiagonalUp).LineStyle = xlNone
    With Selection.Borders(xlEdgeLeft)
        .LineStyle = xlContinuous
        .ColorIndex = 0
        .TintAndShade = 0
        .Weight = xlThin
    End With
    With Selection.Borders(xlEdgeTop)
        .LineStyle = xlContinuous
        .ColorIndex = 0
        .TintAndShade = 0
        .Weight = xlThin
    End With
    With Selection.Borders(xlEdgeBottom)
        .LineStyle = xlContinuous
        .ColorIndex = 0
        .TintAndShade = 0
        .Weight = xlThin
    End With
    With Selection.Borders(xlEdgeRight)
        .LineStyle = xlContinuous
        .ColorIndex = 0
        .TintAndShade = 0
        .Weight = xlThin
    End With
    Selection.Borders(xlInsideVertical).LineStyle = xlNone
    Selection.Borders(xlInsideHorizontal).LineStyle = xlNone

    End If
Next rch

Always use Option Explicit and declare your variables as close as possible to their first use.
Fully qualify your worksheet references (see #5)

When calculating your LastRowMatchC, always make sure to fully qualify ALL worksheet references like this

Worksheets("Compliance").Cells(Worksheets("Compliance").Rows.Count, 1).End(xlUp).Row

or

With Worksheets("Compliance")
    LastRowMatchC = .Cells(.Rows.Count, 1).End(xlUp).Row
End With

(Notice the dot in front of the Rows) Otherwise, the the Rows.Count is looking at the currently active worksheet and not the one you intended.

Use EnableEvents and ScreenUpdating when you're making changes directly on the worksheet.

You can turn off events and screen updating before and after your loop to give a big speed boost to the reformatting:

Application.EnableEvents = False
Application.ScreenUpdating = False

For rc = rngc.Count To 1 Step -1
    '--- do your thing here
Next rc

Application.EnableEvents = True
Application.ScreenUpdating = True

(More on this in the next comment)

Avoid using Select and try to define any constant "magic values" in an expression. No one knows why you're looking for the value "2" here (and you may not remember a year from now). So replace the MAGIC_VALUE name with something meaningful to your application.

A partial example using your code:

Option Explicit

Sub InsertRows()
    Dim lastRow As Long
    With Worksheets("Compliance")
        lastRow = .Cells(.Rows.Count, "S").End(xlUp).Row
    End With

    Dim rngc As Range
    Set rngc = Worksheets("Compliance").Range("S8:S" & lastRow)

    AppPerformance SetTo:=False

    Const MAGIC_VALUE As Long = 2

    Dim rc As Long
    For rc = rngc.Rows.Count To 1 Step -1
        If rngc(rc).Value = MAGIC_VALUE Then
            rngc(rc + 1).EntireRow.Insert
            Dim newRow As Range
            Set newRow = rngc(rc + 1).EntireRow
            With newRow
                With .Interior
                    .Pattern = xlSolid
                    .PatternColorIndex = xlAutomatic
                    .ThemeColor = xlThemeColorAccent1
                    .TintAndShade = 0.599993896298105
                    .PatternTintAndShade = 0
                End With
                .Borders(xlDiagonalDown).LineStyle = xlNone
                .Borders(xlDiagonalUp).LineStyle = xlNone
                With .Borders(xlEdgeLeft)
                    .LineStyle = xlContinuous
                    .ColorIndex = 0
                    .TintAndShade = 0
                    .Weight = xlThin
                End With
                '--- keep going with formatting ...
            End With
        End If
    Next rc

    AppPerformance SetTo:=True

End Sub

Private Sub AppPerformance(ByVal SetTo As Boolean)
    With Application
        .EnableEvents = SetTo
        .ScreenUpdating = SetTo
    End With
End Sub


SQL Injection when not using variables Do I still need to protect my code against SQL injection when I'm not using variables in SQL queries?

Is this code still vulnerable?:

$result = mysqli_query($connect,"SELECT subsubcat_name FROM subsubcategories WHERE subcat_ID = 4");
while ($row=mysqli_fetch_array($result)){
echo $row['subsubcat_name'];
echo '<br>';
}

Since the SQL query is just a fixed string, there is no chance for SQL injection.

However, the way you print the output is a concern, because you fail to escape it for HTML.  For example, if one of the subsubcat_names contains a < character, then it might be interpreted as an HTML tag.

In the worst case, if subsubcat_name contains a malicious string like <script>while (true) alert("Ha ha");</script>, it would be executed by the browser as JavaScript.  Cross-site scripting attacks like that can be used, for example, to steal login credentials.

To prevent such accidental or malicious breakage, you should call htmlspecialchars():

echo htmlspecialchars($row['subsubcat_name']);

Even if the subsubcat_names are not under user control, it's still good programming practice to ensure correctness by escaping all HTML output as HTML.


Another stack implementation (in C) I just started learning C and the online book contained the exercise 'implement a stack'. So I did, but thought I'd put it here, because I still don't feel comfortable with pointers.

So here it is:

#include <stdlib.h>
#include <stdio.h>

struct StackItem {
    struct StackItem *previous;
    int value;
};

void stack_push(struct StackItem **current, int value) {
    struct StackItem *tmp = malloc(sizeof(struct StackItem));
    tmp->previous = *current;
    tmp->value = value;
    *current = tmp;
}

void stack_pop(struct StackItem **current) {
    if ((*current)->previous != NULL) {
        struct StackItem *tmp = (*current)->previous;
        free(*current);
        *current = tmp;
    } else printf("%s\n", "Previous is NULL");
}

void stack_print(struct StackItem *current) {
    printf("%d\n", current->value);
    while (current->previous != NULL) {
        current = current->previous;
        printf("%d\n", current->value);
    }
}

int main(int argc, char **argv) {
    struct StackItem *current = malloc(sizeof(struct StackItem));
    current->value = 2147483647;
    current->previous = NULL;
    char input[10];
    int i = 0;
    while (i < 10) {
        puts("Enter integer value to add to the stack:");
        gets(input);
        char *end;
        int newVal = (int)strtol(input, &end, 10);
        if (*end != '\0') { puts("That was no integer value."); --i; }
        else stack_push(&current, newVal);
        ++i;
    }

    stack_print(current);
    puts("Stack printed once.");

    i = 0;
    while (i < 10) {
        stack_pop(&current);
        i++;
    }

    stack_print(current);
    puts("Stack printed twice.");

    return 0;
}

Is there anything wrong/not good (I mean two different things here) with this?

Your code looks nice and compiles cleanly - good signs.  Some comments, nevertheless:

all functions except main should be declared static
the opening brace { for a function should be in column 0
stack_print should take a const parameter, as you are not modifying the stack:

 void stack_print(const struct StackItem *current)

the stack variable name current would be better as (for example) stack
you define the stack current by allocating a StackItem but never use
this item.  It would be better to define the stack using just a pointer:

 struct StackItem *current = NULL;

this involves some changes to other functions because the 'empty stack'
condition is no longer (*current)->previous == NULL but simply *current
== NULL.   This affects stack_pop and stack_print.  The latter becomes:

void stack_print(const struct StackItem *current)
{
    while (current != NULL) {
        printf("%d\n", current->value);
        current = current->previous;
    }
}

for-loops are often better than while loops - eg when you are counting
through a range.  And the loop variable is best declared in the loop where
possible:

for (int i = 0; i < 10; ++i) {
    stack_pop(&current);
}

using constants in the code like 10 is often a bad idea.  Some are fine (eg
the 10 in the strtol call, but the loops would be better having a #defined
limit

#define N_VALUES  10

for (int i = 0; i < N_VALUES; ++i) {...}

Such #defines go at the top of the file after the #includes
gets is not safe and should not be used.  Using gets with a buffer of 10
bytes, if the user types in more than 10 characters the buffer overflows and
overwrites the call stack (not your stack but the process stack).  Use
fgets instead:

fgets(input, (int) sizeof input, stdin);

there is no error handling, but that is ok for an exercise.  In a real
program, you would want to check that malloc does not fail (using perror
to print an error message if it does - an then you have to decide what to do
about the failure) and you might want to return success or failure from
the push/pop functions.


Bloom Filter in Haskell I recently implemented a Bloom Filter in Haskell and, although I am not new to functional programming, I am a beginner when it comes to Haskell itself.

I'll gladly take any feedback regarding the implementation or the code style. I tried to stick to the Haskell conventions regarding function and variable naming, but I may have gotten some stuff wrong.

You will notice that I'm using my own implementation of a Bitset, you can assume it behaves like a normal one (I sure hope so).

module DataStructures.BloomFilter (empty, insert, member, DataStructures.BloomFilter.null) where

import System.Random (random, StdGen)
import qualified DataStructures.Bitset as Bitset
import Data.Hashable (hashWithSalt)

-- n: expected number of items in the Bloom Filter
-- p: acceptable probability of a false positive
-- m: max number of bits the Bloom Filter will use
-- k: number of hashing functions

data BloomFilter = BloomFilter {
  n :: Int, p :: Float, bitset :: Bitset.Bitset, m :: Int, k :: Int, hashSeed :: Int
} deriving (Eq, Show)

getMaxSize :: Int -> Float -> Int
getMaxSize n p = abs $ ceiling $ fromIntegral n * (log p) / (log (1 / (log 2 ^ 2)))

getNumberOfHashFunctions :: Int -> Int -> Int
getNumberOfHashFunctions n m = round $ fromIntegral (m `div` n) * log 2

empty :: Int -> Float -> StdGen -> BloomFilter
empty n p randGen =
  let m = getMaxSize n p
      k = getNumberOfHashFunctions n m
      hashSeed = fst $ random randGen
  in  BloomFilter n p Bitset.empty m k hashSeed

null :: BloomFilter -> Bool
null = Bitset.null . bitset

getHashes :: Show a => BloomFilter -> a -> [Int]
getHashes bloomFilter elem =
  let str     = show elem
      seed    = hashSeed bloomFilter
      maxSize = m bloomFilter
  in  (`mod` maxSize) . abs . (`hashWithSalt` str) . (seed +) <$> [1..(k bloomFilter)]

insert :: Show a => BloomFilter -> a -> BloomFilter
insert bloomFilter elem =
  let hashes    = getHashes bloomFilter elem
      newBitset = Bitset.insertMany (bitset bloomFilter) hashes
  in  bloomFilter { bitset = newBitset }

-- Returns whether an element *may be* present in the bloom filter.
-- This function can yield false positives, but not false negatives.
member :: Show a => BloomFilter -> a -> Bool
member bloomFilter elem =
  let hashes = getHashes bloomFilter elem
      bs     = bitset bloomFilter
  in  all (Bitset.member bs) hashes

I would have liked to use a murmur3 hashing function but all Haskell implementations use types that I'm not yet familiar with (Word32, ByteString).

Instead of transforming everything to String using Show just for the purpose of hashing it, you should constraint element types to be Hashable instead:

getHashes :: Hashable a => BloomFilter -> a -> [Int]
getHashes bloomFilter elem =
  let seed    = hashSeed bloomFilter
      maxSize = m bloomFilter
  in  (`mod` maxSize) . abs . (`hashWithSalt` elem) . (seed +) <$> [1..(k bloomFilter)]

I'd also use maxSize and numFuns as the field names in BloomFilter and then use RecordWildCards:

getHashes :: Hashable a => BloomFilter -> a -> [Int]
getHashes BloomFilter{..} elem = map nthHash [1..numFuns]
  where
    nthHash n = abs (hashWithSalt (n + hashSeed) elem) `mod` maxSize

Or maybe even nicer:

getHashes :: Hashable a => BloomFilter -> a -> [Int]
getHashes BloomFilter{..} elem =
  [ abs (hashWithSalt (i + hashSeed) elem) `mod` maxSize  | i <- [1..numFuns] ]


Homework / toy program - character matching Given a string A = "Hello world" and another string B = "wor", I have to print ("TRUE" or "FALSE") whether the characters in B are present in A. For example, if B = "ollw l" the result should be TRUE; if B = "worlds" the result should be FALSE; if B = "ww", result is FALSE. It has to be done in O(n) time complexity.

My solution is given below. Although this runs in O(n) time I would like some suggestions on how to improve.

int main(void)
{
        char *A = "Hello world";
        char *B = "ollw l";
        int table['z'] = {0};

        while (*A != '\0') {
                table[*A] += 1;
                A++;
        }

        while (*B != '\0') {
                table[*B] -= 1;
                if (table[*B] < 0) {
                        printf("%s\n", "FALSE");
                        return 0;
                }
                B++;
        }
        printf("%s\n", "TRUE");
}

Your algorithm looks alright

The algorithmic idea seems correct and fit for meeting the stated requirements. There are also no unreasonable constant factors or excessive resource usage just to meet the required asymptotic complexity. Overall, I think you did the right thing.

Try to nail down the requirements

I'm not entirely sure what your function is supposed to do. Should it only take into account letters or also other characters like white-space and punctuation? Should it be case-sensitive or insensitive? Either choice is reasonable but one has to be made.

The following item is related to this.

Know your range

You're using an array from 0 (inclusive) to z (exclusive). As others have already pointed out in comments, this means an out of bounds access if the character z occurs in your input which is almost certainly a bug. Adding 1 to the array size might seem to fix the issue but I'd suggest to look more closer at the range of valid inputs.

First, you have to define your range of valid characters. This might be the whole range of the char data type. Which is implementation-defined so you should use the CHAR_MIN and CHAR_MAX macros defined in <limits.h>. (Your input actually can never contain the value 0 because you'd treat it as the sentinel value but wasting that one index seems reasonable.)

If your input range is smaller than that (for example, only ASCII characters or only alphanumeric characters), you should do two things.

Verify that the input is in range and do some reasonable error handling if it is not. If you decide to give your function a narrow contract, invalid inputs mean undefined behavior and you can use the assert macro that is only active in debug builds and gain a little performance in release builds.
Subtract the lowest permissible character from the character at hand to get the index in your array.

For the ASCII character set, the range is [0, 128) and [32, 126) for the printable sub-set. (It would probably be more readable to use ' ' (space) and '~' + 1 in code instead of these magic numbers.)

Be warned that the C standard doesn't require that the character set is ASCII and the ASCII math might be wrong for other encodings. Some encodings also have multi-byte characters and those will cause you never-ending nightmares if you wish to handle them correctly.

Beware of integral promotion

If your input can be the full range of the char data type, there is a nasty culprit here. The C standard doesn't specify whether char is signed or unsigned. If your code is used on an implementation where char is signed, the bytes 0x80 … 0xff will have negative numeric values.

If you compute your indices by subtracting CHAR_MIN, you'll be fine because it will make those values positive again. If you, however, expect values in the range [0, 256) you'll get bad surprises. The usual way to deal with this is to explicitly cast to unsigned char before using the value.

Consider saving some space – or maybe don't

You're currently using an int to count the frequency of a character. This is likely a reasonable choice but I'd like to challenge you to justify it.

If you want to be on the absolute safe side, a size_t will guarantee that you can even deal with the largest possible array, filled with the same character. If you go for size_t, be aware that it is an unsigned type so you cannot check for “less than zero” after the subtraction but have to check for “equal to zero” before subtracting.

If you want to preserve space (and thus get lower cache usage), you might want to reach for a smaller data type but you'll get undefined behavior if its range is exceeded.

Any decision can be justified but you should be aware of its implications. If your function makes assumptions about the maximum frequency of a character, its documentation should state this limitation.

Separate different concerns into different functions

You have put all code into main. This is obviously not a very modular design and hard to unit-test. I'd recommend that you break your logic into at least three functions.

One function that takes two character arrays and checks whether the former contains all characters from the latter. It doesn't print anything but returns a boolean value.
One function that converts a boolean value into a pointer to a statically allocated character array with a human-readable text. This can be as simple as return value ? "true" : "false";.
One function that obtains (or hard-codes) the input, calls the first function to get the result and then prints the human-readable representation obtained from the second function. (You can leave this logic in main if that's all your program does.)