Need someone to "proof read" my library before publishing

I have written a simple library that I have found to be very functional, so I want to publish it to the Arduino repository. However, I’ve never published a library before so I need someone with experience and knowledge to look it over and maybe offer advice for code efficiency or syntax correctness etc.

I’ve read all of the guides concerning publishing and I’ve got the .properties file done as well as the keyword.txt file. The source code is placed in the right folder and I’ve gone through and simplified the code to the best of my abilities.

I also created a very thorough and well-formatted readme.md file.

The library really is pretty simple. My largest method is maybe around 10 lines of code. So I don’t think it would take an experienced coder more than 15 minutes to look it over and maybe spot areas that could be improved.

If you are interested, please let me know here or send me a message privately.

Thank you,

Mike Sims

Posting your email on the internet in plain text like that is pretty foolish. Get ready for the onslaught of spam. You should probably take that down and come here for responses.

Also, if you want someone to look at the code then you might consider posting it. It's hard to proofread the code without actually seeing the code.

Delta_G:
Posting your email on the internet in plain text like that is pretty foolish. Get ready for the onslaught of spam. You should probably take that down and come here for responses.

OK, I removed the email address although I'm not too concerned with spammers fishing an Arduino forum for email addresses... been a network engineer for over 25 years and have my Gmail set up pretty tight. However, I was hesitant at first posting it but decided to anyway ... you re-enforced my concerns so off it goes! :slight_smile:

Delta_G:
Also, if you want someone to look at the code then you might consider posting it. It's hard to proofread the code without actually seeing the code.

That's an awful lot to post here ... it would just be messy ... and I'd prefer to work with someone offline. I'm not looking to post my library in an open forum either, as I'd prefer to wait until it's published to the repository ... for various reasons.

EasyGoing1:
OK, I removed the email address although I'm not too concerned with spammers fishing an Arduino forum for email addresses... been a network engineer for over 25 years and have my Gmail set up pretty tight. However, I was hesitant at first posting it but decided to anyway ... you re-enforced my concerns so off it goes! :slight_smile:

They don't come here specifically, they just crawl the whole of the internet looking for anything that matches a regex that looks like an email address. If you post it, the slime-bag spammers will find it.

EasyGoing1:
That's an awful lot to post here ... it would just be messy ... and I'd prefer to work with someone offline. I'm not looking to post my library in an open forum either, as I'd prefer to wait until it's published to the repository ... for various reasons.

Well that makes zero sense. Why wouldn't you just post it. You yourself said it wasn't that huge. Even if it is, it can't possibly be the biggest piece of code posted here.

At least if you post here you'll get multiple looks from multiple people. A group effort will almost always find more stuff than a single reader. Plus most of the really strong folks would probably rather do it here.

What "various reasons". Are you just afraid that it won't be up to snuff and you look bad or something? If it is destined to be open source and on the Arduino site then I can't think of any good reason not to post it. Or a link to it. Or something.

But whatever. Sit and wait for someone if that's what you want to do. Kind of ridiculous, but whatever.

If you don't want to discuss on the forum then you should probably have this moved to the Gigs and Collaborations section.

Delta_G:
What "various reasons". Are you just afraid that it won't be up to snuff and you look bad or something? If it is destined to be open source and on the Arduino site then I can't think of any good reason not to post it. Or a link to it. Or something.

OK, you've convinced me. I will create a github repository for it and post the link here in a few minutes.

Delta_G:
But whatever. Sit and wait for someone if that's what you want to do. Kind of ridiculous, but whatever.

Well that was unnecessary ... I always cringe when seeking help in this forum because of negativity just like this, which seems to be a staple among this forum specifically ... I don't find such comments to be in any way helpful nor professional ... but definitely deterring.

(deleted)

EasyGoing1:
Well that was unnecessary ... I always cringe when seeking help in this forum because of negativity just like this, which seems to be a staple among this forum specifically ... I don't find such comments to be in any way helpful nor professional ... but definitely deterring.

You read that completely wrong. That was simply a "hey, I'm not going to try to convince you, you do what you want". There was nothing insulting or demeaning or mean about it. If you saw something then that's all in your head.

For someone with the moniker "EasyGoing" you sure are quick to anger.

Delta_G:
For someone with the moniker "EasyGoing" you sure are quick to anger.

More frustration with the negativity than anger. If it wasn't something that I see on these forums consistently, I wouldn't even mention it.

OK, I published the repository

I am currently creating more examples for the examples folder

EasyGoing1:
More frustration with the negativity than anger. If it wasn't something that I see on these forums consistently, I wouldn't even mention it.

I don't see what's negative about it. You obviously wanted to do things one way and I was saying go ahead. What is negative about that?

The usual term is "Code review", BTW.

Ideally, it's preceded by a Design Specification and "Design Review" (says what it's going to do and why), a "Functional Specification" (and review) that says how it's going to do it, and the CR actually checks out the nuts&Bolts of how it's actually written. Oh, and a separate documentation review (there IS documentation, right?) All very ISO9000 and"Funnel, tootsie-roll, plunger" compatible.

It usually doesn't all happen for OSSW, and even less for Arduino libraries. IMNSHO, the biggest weakness is the lack of documentation...

The first thing I see is a bit troubling:

class BlockNot {
public:
    #define Done            BlockNot::passed()
    #define IsDone          BlockNot::passed()
    #define Ran             BlockNot::passed()
    #define HasRun          BlockNot::passed()
    #define Passed          BlockNot::passed()
    #define HasPassed       BlockNot::passed()
    #define Now             BlockNot::passed()
    #define IsNow           BlockNot::passed()
    #define HasNotFinished  BlockNot::notPassed()
    #define IsNotDone       BlockNot::notPassed()
    #define IsNotNow        BlockNot::notPassed()
    #define NotPassed       BlockNot::notPassed()
    #define NotHere         BlockNot::notPassed()
    #define NotNow          BlockNot::notPassed()
    #define Reset           BlockNot::reset()
    #define ResetTimer      BlockNot::reset()
    #define TimeLapsed      BlockNot::timelapsed()
    #define GetTime         BlockNot::timeLapsed()
    #define SetTime         BlockNot::setTime
    #define NewTime         BlockNot::setTime
    #define SetNewTime      BlockNot::setTime
    #define Pause           BlockNot::blockNot()
    #define Wait            BlockNot::blockNot()
    #define Delay           BlockNot::blockNot()
    #define BN              BlockNot

First off, the #define statements don't belong inside the class definition.

But more concerning is that you've broken someone else's code here. You just took a bunch of #define names, some of which are pretty common. Someone shouldn't need to know what's inside your library to use it. And a constant being redefined in a library can be hard to find.

I see what you're doing here, but most libraries don't and there's a good reason. If you insist on having all these macros, give them something to guarantee that they're going to be unique.

#define Now             BlockNot::passed()

I can definitely see someone having Now defined as a function name or a variable name. That's going to happen a lot. Maybe if you called it BN_NOW or something.

Which brings another point. Your macros should, by convention, be all caps. We do that not for the compiler, but for the guy writing the code.

Oh and what's worse is that they all get redefined in the next class definition. It doesn't matter which class you create, only the last #define in the file is going to count. So even if you create an instance of the first class, all the #define will come from the last one.

spycatcher2k:
While I understand YOU may find this unhelpful, think about your post, you kind of said 'Hi - I've written stuff and want you to look at it for free, but I want YOU to work for it by sending me emails, then having a closed discussion', not really in the spirit of open source, is it?

I have no idea, I've never written nor published open-source code before. Hence why I created this post in the first place ... as noted in my original post.

spycatcher2k:
And who do you think is a proffesional here? It's a public forum, NOT run by anyone from Arduino (I'm not even sure if any of the Arduino team browse on a regular basis).

I make an assumption that anyone who is intelligent enough to write C++ code for Arduino has the capacity to both understand the plight of others who lack knowledge, and the ability to communicate without adding comments that are negative in nature, as they serve no purpose towards the primary goal of these forums, which is to offer help to people who need it. I consider the exercise of that behavior to fall under the category of "professional". I was not specifically requiring that anyone who assists here be people who work in a professional environment. Professional behavior can also be considered "classy" or "ethical/moral".

westfw:
IMNSHO, the biggest weakness is the lack of documentation...

I think you'll find the README.md file to be quite thorough.

bool BlockNot::passed() {
    reset();
    return (millis() - startTime) >= duration;
}

Have you tested this?

By having reset there, and in reset it sets startTime to millis, this will always return false unless duration is 0.

bool BlockNot::notPassed() {
    reset();
    return (millis() - startTime) < duration;
}

same problem here. This always returns false unless duration is 0.

void BlockNot::setTime(unsigned long time) {
    if (time < 1) time = 100;
    startTime = time;
    reset();
}

Should that set startTime? or duration?

It is silly to set startTime there and then call reset() which just sets it back to millis.

void BlockNot::blockNot() {
    reset();
    while ((millis() - startTime) < duration) delay(1);
}

I thought the point was to be non-blocking. This is blocking code. Maybe I misunderstand the use of this one.

Delta_G:
Have you tested this?

By having reset there, and in reset it sets startTime to millis, this will always return false unless duration is 0.

I have no idea how the reset(); got put in there. I'm correcting this and re-checking then re-publishing.

Thank you

Me: the biggest weakness is the lack of documentation…

You: OK, I published the repository

Oh! You DO have documentation! Very good!

You just took a bunch of #define names, some of which are pretty common.

Yeah, I agree with the concerns expressed here. “Reset”, and “Delay” too ! (one fundamental principal in many coding standards is that while there are a number of different ways that you might capitalize identifiers, you DO want to be pretty careful that differently capitalized versions of the same word do NOT show up, especially not as meaning very different things.